diff --git a/CHANGELOG.md b/CHANGELOG.md index 22e8665..811eec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +### v1.7.3 + +## Enhancements + +* Overwritten view adapter materialization and made improvements. +* Overwritten table adapter materizalization and made improvements in handling model level constraints +* Made Constraint name mandatory +* Added several macros to manage indexes, dropping table dependencies and managing model level constraints +* Bump dbt-tests-adapter requirement from ~=1.7.3 to ~=1.7.4 +* Bump py-test adapter requirement from ~=pytest==7.4.3 to ~=pytest==7.4.4 +* Bump precommit adapter requirement from ~=pre-commit==3.5.0 to ~=pre-commit==3.6.0 + ### v1.7.2 ## Bug Fixes diff --git a/dbt/adapters/fabric/__version__.py b/dbt/adapters/fabric/__version__.py index 2196826..a26c301 100644 --- a/dbt/adapters/fabric/__version__.py +++ b/dbt/adapters/fabric/__version__.py @@ -1 +1 @@ -version = "1.7.2" +version = "1.7.3" diff --git a/dbt/adapters/fabric/fabric_adapter.py b/dbt/adapters/fabric/fabric_adapter.py index c63c5fd..20629a2 100644 --- a/dbt/adapters/fabric/fabric_adapter.py +++ b/dbt/adapters/fabric/fabric_adapter.py @@ -1,6 +1,7 @@ from typing import List, Optional import agate +import dbt.exceptions from dbt.adapters.base import Column as BaseColumn # from dbt.events.functions import fire_event @@ -202,6 +203,13 @@ def render_model_constraint(cls, constraint: ModelLevelConstraint) -> Optional[s constraint_prefix = "add constraint " column_list = ", ".join(constraint.columns) + if constraint.name is None: + raise dbt.exceptions.DbtDatabaseError( + "Constraint name cannot be empty. Provide constraint name - column " + + column_list + + " and run the project again." + ) + if constraint.type == ConstraintType.unique: return ( constraint_prefix diff --git a/dbt/include/fabric/macros/adapters/indexes.sql b/dbt/include/fabric/macros/adapters/indexes.sql index 11b2527..c4f12a2 100644 --- a/dbt/include/fabric/macros/adapters/indexes.sql +++ b/dbt/include/fabric/macros/adapters/indexes.sql @@ -18,8 +18,36 @@ {# {% exceptions.raise_compiler_error('Indexes are not supported') %} #} {% endmacro %} -{% macro drop_all_indexes_on_table() -%} - {# {% exceptions.raise_compiler_error('Indexes are not supported') %} #} +{% macro drop_fk_indexes_on_table(relation) -%} + {% call statement('find_references', fetch_result=true) %} + USE [{{ relation.database }}]; + SELECT obj.name AS FK_NAME, + sch.name AS [schema_name], + tab1.name AS [table], + col1.name AS [column], + tab2.name AS [referenced_table], + col2.name AS [referenced_column] + FROM sys.foreign_key_columns fkc + INNER JOIN sys.objects obj + ON obj.object_id = fkc.constraint_object_id + INNER JOIN sys.tables tab1 + ON tab1.object_id = fkc.parent_object_id + INNER JOIN sys.schemas sch + ON tab1.schema_id = sch.schema_id + INNER JOIN sys.columns col1 + ON col1.column_id = parent_column_id AND col1.object_id = tab1.object_id + INNER JOIN sys.tables tab2 + ON tab2.object_id = fkc.referenced_object_id + INNER JOIN sys.columns col2 + ON col2.column_id = referenced_column_id AND col2.object_id = tab2.object_id + WHERE sch.name = '{{ relation.schema }}' and tab2.name = '{{ relation.identifier }}' + {% endcall %} + {% set references = load_result('find_references')['data'] %} + {% for reference in references -%} + {% call statement('main') -%} + alter table [{{reference[1]}}].[{{reference[2]}}] drop constraint [{{reference[0]}}] + {%- endcall %} + {% endfor %} {% endmacro %} {% macro create_clustered_index(columns, unique=False) -%} @@ -29,3 +57,38 @@ {% macro create_nonclustered_index(columns, includes=False) %} {# {% exceptions.raise_compiler_error('Indexes are not supported') %} #} {% endmacro %} + +{% macro fabric__list_nonclustered_rowstore_indexes(relation) -%} + {% call statement('list_nonclustered_rowstore_indexes', fetch_result=True) -%} + + SELECT i.name AS index_name + , i.name + '__dbt_backup' as index_new_name + , COL_NAME(ic.object_id,ic.column_id) AS column_name + FROM sys.indexes AS i + INNER JOIN sys.index_columns AS ic + ON i.object_id = ic.object_id AND i.index_id = ic.index_id and i.type <> 5 + WHERE i.object_id = OBJECT_ID('{{ relation.schema }}.{{ relation.identifier }}') + + UNION ALL + + SELECT obj.name AS index_name + , obj.name + '__dbt_backup' as index_new_name + , col1.name AS column_name + FROM sys.foreign_key_columns fkc + INNER JOIN sys.objects obj + ON obj.object_id = fkc.constraint_object_id + INNER JOIN sys.tables tab1 + ON tab1.object_id = fkc.parent_object_id + INNER JOIN sys.schemas sch + ON tab1.schema_id = sch.schema_id + INNER JOIN sys.columns col1 + ON col1.column_id = parent_column_id AND col1.object_id = tab1.object_id + INNER JOIN sys.tables tab2 + ON tab2.object_id = fkc.referenced_object_id + INNER JOIN sys.columns col2 + ON col2.column_id = referenced_column_id AND col2.object_id = tab2.object_id + WHERE sch.name = '{{ relation.schema }}' and tab1.name = '{{ relation.identifier }}' + + {% endcall %} + {{ return(load_result('list_nonclustered_rowstore_indexes').table) }} +{% endmacro %} diff --git a/dbt/include/fabric/macros/adapters/metadata.sql b/dbt/include/fabric/macros/adapters/metadata.sql index ed3a2aa..e5dc3a1 100644 --- a/dbt/include/fabric/macros/adapters/metadata.sql +++ b/dbt/include/fabric/macros/adapters/metadata.sql @@ -27,7 +27,6 @@ {% endmacro %} {% macro fabric__list_relations_without_caching(schema_relation) -%} -{{ log("schema_relation in fabric__list_relations_without_caching is " ~ schema_relation.schema, info=True) }} {% call statement('list_relations_without_caching', fetch_result=True) -%} select table_catalog as [database], @@ -44,8 +43,25 @@ {{ return(load_result('list_relations_without_caching').table) }} {% endmacro %} +{% macro fabric__get_relation_without_caching(schema_relation) -%} + {% call statement('list_relations_without_caching', fetch_result=True) -%} + select + table_catalog as [database], + table_name as [name], + table_schema as [schema], + case when table_type = 'BASE TABLE' then 'table' + when table_type = 'VIEW' then 'view' + else table_type + end as table_type + + from INFORMATION_SCHEMA.TABLES {{ information_schema_hints() }} + where table_schema like '{{ schema_relation.schema }}' + and table_name like '{{ schema_relation.identifier }}' + {% endcall %} + {{ return(load_result('list_relations_without_caching').table) }} +{% endmacro %} + {% macro fabric__get_relation_last_modified(information_schema, relations) -%} -{{ log("information_schema - "~ information_schema, info=True) }} {%- call statement('last_modified', fetch_result=True) -%} select o.name as [identifier] diff --git a/dbt/include/fabric/macros/adapters/relation.sql b/dbt/include/fabric/macros/adapters/relation.sql index 585fb83..380d131 100644 --- a/dbt/include/fabric/macros/adapters/relation.sql +++ b/dbt/include/fabric/macros/adapters/relation.sql @@ -39,7 +39,38 @@ path={"schema": reference[0], "identifier": reference[1]})) }} {% endfor %} {% elif relation.type == 'table'%} + {# {% call statement('find_references', fetch_result=true) %} + USE [{{ relation.database }}]; + SELECT obj.name AS FK_NAME, + sch.name AS [schema_name], + tab1.name AS [table], + col1.name AS [column], + tab2.name AS [referenced_table], + col2.name AS [referenced_column] + FROM sys.foreign_key_columns fkc + INNER JOIN sys.objects obj + ON obj.object_id = fkc.constraint_object_id + INNER JOIN sys.tables tab1 + ON tab1.object_id = fkc.parent_object_id + INNER JOIN sys.schemas sch + ON tab1.schema_id = sch.schema_id + INNER JOIN sys.columns col1 + ON col1.column_id = parent_column_id AND col1.object_id = tab1.object_id + INNER JOIN sys.tables tab2 + ON tab2.object_id = fkc.referenced_object_id + INNER JOIN sys.columns col2 + ON col2.column_id = referenced_column_id AND col2.object_id = tab2.object_id + WHERE sch.name = '{{ relation.schema }}' and tab2.name = '{{ relation.identifier }}' + {% endcall %} + {% set references = load_result('find_references')['data'] %} + {% for reference in references -%} + -- dropping referenced table {{ reference[0] }}.{{ reference[1] }} + {{ fabric__drop_relation_script(relation.incorporate( + type="table", + path={"schema": reference[1], "identifier": reference[2]})) }} + {% endfor %} #} {% set object_id_type = 'U' %} + {%- else -%} {{ exceptions.raise_not_implemented('Invalid relation being dropped: ' ~ relation) }} {% endif %} diff --git a/dbt/include/fabric/macros/materializations/models/table/columns_spec_ddl.sql b/dbt/include/fabric/macros/materializations/models/table/columns_spec_ddl.sql index 341e9b5..83188c9 100644 --- a/dbt/include/fabric/macros/materializations/models/table/columns_spec_ddl.sql +++ b/dbt/include/fabric/macros/materializations/models/table/columns_spec_ddl.sql @@ -1,16 +1,22 @@ -{% macro fabric__table_columns_and_constraints(relation) %} +{% macro fabric__build_columns_constraints(relation) %} {# loop through user_provided_columns to create DDL with data types and constraints #} {%- set raw_column_constraints = adapter.render_raw_columns_constraints(raw_columns=model['columns']) -%} - {%- set raw_model_constraints = adapter.render_raw_model_constraints(raw_constraints=model['constraints']) -%} ( {% for c in raw_column_constraints -%} {{ c }}{{ "," if not loop.last }} {% endfor %} ) +{% endmacro %} + +{% macro fabric__build_model_constraints(relation) %} + {# loop through user_provided_columns to create DDL with data types and constraints #} + {%- set raw_model_constraints = adapter.render_raw_model_constraints(raw_constraints=model['constraints']) -%} {% for c in raw_model_constraints -%} {% set alter_table_script %} alter table {{ relation.include(database=False) }} {{c}}; {%endset%} - EXEC('{{alter_table_script}};') + {% call statement('alter_table_add_constraint') -%} + {{alter_table_script}} + {%- endcall %} {% endfor -%} {% endmacro %} diff --git a/dbt/include/fabric/macros/materializations/models/table/create_table_as.sql b/dbt/include/fabric/macros/materializations/models/table/create_table_as.sql index 4ac5146..2ec9013 100644 --- a/dbt/include/fabric/macros/materializations/models/table/create_table_as.sql +++ b/dbt/include/fabric/macros/materializations/models/table/create_table_as.sql @@ -4,7 +4,6 @@ path={"identifier": relation.identifier.replace("#", "") ~ '_temp_view'}, type='view')-%} {% do run_query(fabric__drop_relation_script(tmp_relation)) %} - {% do run_query(fabric__drop_relation_script(relation)) %} {% set contract_config = config.get('contract') %} @@ -12,7 +11,7 @@ {% if contract_config.enforced %} CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] - {{ fabric__table_columns_and_constraints(relation) }} + {{ fabric__build_columns_constraints(relation) }} {{ get_assert_columns_equivalent(sql) }} {% set listColumns %} diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql new file mode 100644 index 0000000..908023b --- /dev/null +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -0,0 +1,41 @@ +{% materialization table, adapter='fabric' %} + + -- Create target relation + {%- set target_relation = this.incorporate(type='table') %} + -- grab current tables grants config for comparision later on + {% set grant_config = config.get('grants') %} + {%- set backup_relation = make_backup_relation(target_relation, 'table') -%} + -- drop the temp relations if they exist already in the database + {{ drop_relation_if_exists(backup_relation) }} + -- Rename target relation as backup relation + {%- set relation = fabric__get_relation_without_caching(target_relation) %} + {% if relation|length > 0 %} + {{ adapter.rename_relation(target_relation, backup_relation) }} + {% endif %} + + {{ run_hooks(pre_hooks, inside_transaction=False) }} + -- `BEGIN` happens here: + {{ run_hooks(pre_hooks, inside_transaction=True) }} + + -- build model + {% call statement('main') -%} + {{ get_create_table_as_sql(False, target_relation, sql) }} + {%- endcall %} + + -- cleanup + {{ run_hooks(post_hooks, inside_transaction=True) }} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% do persist_docs(target_relation, model) %} + -- `COMMIT` happens here + {{ adapter.commit() }} + + -- finally, drop the foreign key references if exists + {{ drop_fk_indexes_on_table(backup_relation) }} + -- drop existing/backup relation after the commit + {{ drop_relation_if_exists(backup_relation) }} + -- Add constraints including FK relation. + {{ fabric__build_model_constraints(target_relation) }} + {{ run_hooks(post_hooks, inside_transaction=False) }} + {{ return({'relations': [target_relation]}) }} + +{% endmaterialization %} diff --git a/dbt/include/fabric/macros/materializations/models/view/create_view_as.sql b/dbt/include/fabric/macros/materializations/models/view/create_view_as.sql index 0cfdd06..cfb610f 100644 --- a/dbt/include/fabric/macros/materializations/models/view/create_view_as.sql +++ b/dbt/include/fabric/macros/materializations/models/view/create_view_as.sql @@ -5,6 +5,7 @@ {% macro fabric__create_view_exec(relation, sql) -%} {%- set temp_view_sql = sql.replace("'", "''") -%} + USE [{{ relation.database }}]; {% set contract_config = config.get('contract') %} {% if contract_config.enforced %} diff --git a/dbt/include/fabric/macros/materializations/models/view/view.sql b/dbt/include/fabric/macros/materializations/models/view/view.sql new file mode 100644 index 0000000..30444cd --- /dev/null +++ b/dbt/include/fabric/macros/materializations/models/view/view.sql @@ -0,0 +1,40 @@ +{% materialization view, adapter='fabric' -%} + + {%- set existing_relation = load_cached_relation(this) -%} + {%- set target_relation = this.incorporate(type='view') -%} + + -- make back up relation + {%- set backup_relation_type = 'view' if existing_relation is none else existing_relation.type -%} + {%- set backup_relation = make_backup_relation(target_relation, backup_relation_type) -%} + + {% set grant_config = config.get('grants') %} + {{ run_hooks(pre_hooks, inside_transaction=False) }} + + -- drop target relation if exists already in the database + {{ drop_relation_if_exists(backup_relation) }} + + {%- set relation = fabric__get_relation_without_caching(target_relation) %} + {% if relation|length > 0 %} + {{ adapter.rename_relation(target_relation, backup_relation) }} + {% endif %} + + -- `BEGIN` happens here: + {{ run_hooks(pre_hooks, inside_transaction=True) }} + + -- build model + {% call statement('main') -%} + {{ get_create_view_as_sql(target_relation, sql) }} + {%- endcall %} + + {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %} + {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + + {% do persist_docs(target_relation, model) %} + {{ run_hooks(post_hooks, inside_transaction=True) }} + {{ adapter.commit() }} + {{ drop_relation_if_exists(backup_relation) }} + {{ run_hooks(post_hooks, inside_transaction=False) }} + + {{ return({'relations': [target_relation]}) }} + +{%- endmaterialization -%} diff --git a/dev_requirements.txt b/dev_requirements.txt index c4173cd..afc63a1 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,9 +1,9 @@ -pytest==7.4.3 +pytest==7.4.4 twine==4.0.2 wheel==0.42 -pre-commit==3.5.0 +pre-commit==3.6.0 pytest-dotenv==0.5.2 -dbt-tests-adapter~=1.7.3 +dbt-tests-adapter~=1.7.4 flaky==3.7.0 pytest-xdist==3.5.0 -e . diff --git a/tests/functional/adapter/test_constraints.py b/tests/functional/adapter/test_constraints.py index 4a94c64..eeaef69 100644 --- a/tests/functional/adapter/test_constraints.py +++ b/tests/functional/adapter/test_constraints.py @@ -482,7 +482,7 @@ def models(self): @pytest.fixture(scope="class") def expected_sql(self): return """ -EXEC('create view as -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day;'); CREATE TABLE ( id int not null, color varchar(100), date_day varchar(100) ) EXEC(' alter table add constraint primary key nonclustered(id) not enforced; ;') EXEC(' alter table add constraint foreign key(id) references (id) not enforced; ;') EXEC(' alter table add constraint unique nonclustered(id) not enforced; ;') INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM EXEC('DROP view IF EXISTS +EXEC('create view as -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day;'); CREATE TABLE ( id int not null, color varchar(100), date_day varchar(100) ) INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM EXEC('DROP view IF EXISTS """ def test__constraints_ddl(self, project, expected_sql): @@ -544,7 +544,7 @@ def models(self): @pytest.fixture(scope="class") def expected_sql(self): return """ -EXEC('create view as -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day;'); CREATE TABLE ( id int not null, color varchar(100), date_day varchar(100) ) EXEC(' alter table add constraint primary key nonclustered(id) not enforced; ;') EXEC(' alter table add constraint unique nonclustered(color, date_day) not enforced; ;') EXEC(' alter table add constraint foreign key(id) references (id) not enforced; ;') INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM EXEC('DROP view IF EXISTS +EXEC('create view as -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day;'); CREATE TABLE ( id int not null, color varchar(100), date_day varchar(100) ) INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM EXEC('DROP view IF EXISTS """ def test__model_constraints_ddl(self, project, expected_sql): @@ -612,7 +612,8 @@ def test__constraints_enforcement_rollback( # Verify the previous table still exists relation = relation_from_name(project.adapter, "my_model") - old_model_exists_sql = f"select * from {relation}" + model_backup = str(relation).replace("my_model", "my_model__dbt_backup") + old_model_exists_sql = f"select * from {model_backup}" old_model_exists = project.run_sql(old_model_exists_sql, fetch="all") assert len(old_model_exists) == 1 assert old_model_exists[0][1] == expected_color