From e886a693d2e6970dc2b4147d51b7490f83a119a4 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Tue, 21 May 2024 06:58:20 -0700 Subject: [PATCH 01/17] Updating dispatching methods to ensure dbt-synapse adapter can use adapter dispatch methods --- .../models/incremental/incremental.sql | 14 +++++++------- .../macros/materializations/models/table/clone.sql | 2 +- .../models/table/create_table_as.sql | 6 +++--- .../macros/materializations/models/view/view.sql | 6 +++--- .../macros/materializations/snapshots/helpers.sql | 2 +- .../macros/materializations/snapshots/snapshot.sql | 6 +----- 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index 797f09a..70a4025 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -3,7 +3,7 @@ {%- set full_refresh_mode = (should_full_refresh()) -%} {% set target_relation = this.incorporate(type='table') %} - {%- set relations_list = fabric__get_relation_without_caching(target_relation) -%} + {%- set relations_list = get_relation_without_caching(target_relation) -%} {%- set existing_relation = none %} {% if (relations_list|length == 1) and (relations_list[0][2] == target_relation.schema) @@ -14,9 +14,9 @@ {% set existing_relation = get_or_create_relation(relations_list[0][0], relations_list[0][2] , relations_list[0][1] , relations_list[0][3])[1] %} {% endif %} - {{ log("Full refresh mode" ~ full_refresh_mode)}} + {# {{ log("Full refresh mode" ~ full_refresh_mode)}} {{ log("existing relation : "~existing_relation ~ " type "~ existing_relation.type ~ " is view? "~existing_relation.is_view) }} - {{ log("target relation: " ~target_relation ~ " type "~ target_relation.type ~ " is view? "~target_relation.is_view) }} + {{ log("target relation: " ~target_relation ~ " type "~ target_relation.type ~ " is view? "~target_relation.is_view) }} #} -- configs {%- set unique_key = config.get('unique_key') -%} @@ -31,7 +31,7 @@ {% if existing_relation is none %} {%- call statement('main') -%} - {{ fabric__create_table_as(False, target_relation, sql)}} + {{ create_table_as(False, target_relation, sql)}} {%- endcall -%} {% elif existing_relation.is_view %} @@ -40,19 +40,19 @@ {{ log("Dropping relation " ~ target_relation ~ " because it is a view and this model is a table.") }} {{ drop_relation_if_exists(existing_relation) }} {%- call statement('main') -%} - {{ fabric__create_table_as(False, target_relation, sql)}} + {{ create_table_as(False, target_relation, sql)}} {%- endcall -%} {% elif full_refresh_mode %} {%- call statement('main') -%} - {{ fabric__create_table_as(False, target_relation, sql)}} + {{ create_table_as(False, target_relation, sql)}} {%- endcall -%} {% else %} {%- call statement('create_tmp_relation') -%} - {{ fabric__create_table_as(True, temp_relation, sql)}} + {{ create_table_as(True, temp_relation, sql)}} {%- endcall -%} {% do adapter.expand_target_column_types( from_relation=temp_relation, diff --git a/dbt/include/fabric/macros/materializations/models/table/clone.sql b/dbt/include/fabric/macros/materializations/models/table/clone.sql index 110dd03..b2e9d24 100644 --- a/dbt/include/fabric/macros/materializations/models/table/clone.sql +++ b/dbt/include/fabric/macros/materializations/models/table/clone.sql @@ -24,7 +24,7 @@ {%- set target_relation = this.incorporate(type='table') -%} {% call statement('main') %} - {{ fabric__drop_relation_script(target_relation) }} + {{ drop_relation_if_exists(target_relation) }} {{ create_or_replace_clone(target_relation, defer_relation) }} {% endcall %} {{ return({'relations': [target_relation]}) }} 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 2ec9013..4db19c2 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 @@ -3,11 +3,11 @@ {% set tmp_relation = relation.incorporate( path={"identifier": relation.identifier.replace("#", "") ~ '_temp_view'}, type='view')-%} - {% do run_query(fabric__drop_relation_script(tmp_relation)) %} + {% do run_query(drop_relation_if_exists(tmp_relation)) %} {% set contract_config = config.get('contract') %} - {{ fabric__create_view_as(tmp_relation, sql) }} + {{ create_view_as(tmp_relation, sql) }} {% if contract_config.enforced %} CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] @@ -27,6 +27,6 @@ EXEC('CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] AS (SELECT * FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]);'); {% endif %} - {{ fabric__drop_relation_script(tmp_relation) }} + {{ drop_relation_if_exists(tmp_relation) }} {% endmacro %} diff --git a/dbt/include/fabric/macros/materializations/models/view/view.sql b/dbt/include/fabric/macros/materializations/models/view/view.sql index 8576b2d..74011c2 100644 --- a/dbt/include/fabric/macros/materializations/models/view/view.sql +++ b/dbt/include/fabric/macros/materializations/models/view/view.sql @@ -3,15 +3,15 @@ {%- set target_relation = this.incorporate(type='view') -%} {{log("Target Relation "~target_relation)}} - {%- set relation = fabric__get_relation_without_caching(this) %} + {%- set relation = get_relation_without_caching(this) %} {% set existing_relation = none %} {% if (relation|length == 1) %} {% set existing_relation = get_or_create_relation(relation[0][0], relation[0][2] , relation[0][1] , relation[0][3])[1] %} {% endif %} - {{log("Existing Relation "~existing_relation)}} + {# {{log("Existing Relation "~existing_relation)}} #} {%- set backup_relation = none %} - {{log("Existing Relation type is "~ existing_relation.type)}} + {# {{log("Existing Relation type is "~ existing_relation.type)}} #} {% if (existing_relation != none and existing_relation.type == "table") %} {%- set backup_relation = make_backup_relation(target_relation, 'table') -%} {% elif (existing_relation != none and existing_relation.type == "view") %} diff --git a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql index 58107d4..ca12361 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql @@ -190,7 +190,7 @@ {% macro build_snapshot_staging_table(strategy, temp_snapshot_relation, target_relation) %} {% set temp_relation = make_temp_relation(target_relation) %} - {% set select = fabric__snapshot_staging_table(strategy, temp_snapshot_relation, target_relation) %} + {% set select = snapshot_staging_table(strategy, temp_snapshot_relation, target_relation) %} {% call statement('build_snapshot_staging_relation') %} {{ create_table_as(True, temp_relation, select) }} {% endcall %} diff --git a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql index 83e3f9b..039ae61 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql @@ -75,15 +75,13 @@ insert_cols = quoted_source_columns ) %} - {% endif %} {% call statement('main') %} {{ final_sql }} {% endcall %} - fabric__drop_relation_script(temp_snapshot_relation) - + {{ drop_relation_if_exists(temp_snapshot_relation) }} {% set should_revoke = should_revoke(target_relation_exists, full_refresh_mode=False) %} {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} @@ -94,7 +92,6 @@ {% endif %} {{ run_hooks(post_hooks, inside_transaction=True) }} - {{ adapter.commit() }} {% if staging_table is defined %} @@ -102,7 +99,6 @@ {% endif %} {{ run_hooks(post_hooks, inside_transaction=False) }} - {{ return({'relations': [target_relation]}) }} {% endmaterialization %} From eb7e3f8e77bcd9804dbbf6e361f439ef144c37cd Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Tue, 21 May 2024 09:42:59 -0700 Subject: [PATCH 02/17] additional changes --- dbt/adapters/fabric/__version__.py | 2 +- .../models/incremental/incremental.sql | 10 +++++----- .../materializations/models/table/create_table_as.sql | 2 +- .../macros/materializations/models/view/view.sql | 2 +- .../macros/materializations/snapshots/helpers.sql | 2 +- .../macros/materializations/snapshots/snapshot.sql | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dbt/adapters/fabric/__version__.py b/dbt/adapters/fabric/__version__.py index cb9f516..fd11d27 100644 --- a/dbt/adapters/fabric/__version__.py +++ b/dbt/adapters/fabric/__version__.py @@ -1 +1 @@ -version = "1.8.5" +version = "1.8.6" diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index 70a4025..83e8207 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -3,7 +3,7 @@ {%- set full_refresh_mode = (should_full_refresh()) -%} {% set target_relation = this.incorporate(type='table') %} - {%- set relations_list = get_relation_without_caching(target_relation) -%} + {%- set relations_list = fabric__get_relation_without_caching(target_relation) -%} {%- set existing_relation = none %} {% if (relations_list|length == 1) and (relations_list[0][2] == target_relation.schema) @@ -31,7 +31,7 @@ {% if existing_relation is none %} {%- call statement('main') -%} - {{ create_table_as(False, target_relation, sql)}} + {{ get_create_table_as_sql(False, target_relation, sql)}} {%- endcall -%} {% elif existing_relation.is_view %} @@ -40,19 +40,19 @@ {{ log("Dropping relation " ~ target_relation ~ " because it is a view and this model is a table.") }} {{ drop_relation_if_exists(existing_relation) }} {%- call statement('main') -%} - {{ create_table_as(False, target_relation, sql)}} + {{ get_create_table_as_sql(False, target_relation, sql)}} {%- endcall -%} {% elif full_refresh_mode %} {%- call statement('main') -%} - {{ create_table_as(False, target_relation, sql)}} + {{ get_create_table_as_sql(False, target_relation, sql)}} {%- endcall -%} {% else %} {%- call statement('create_tmp_relation') -%} - {{ create_table_as(True, temp_relation, sql)}} + {{ get_create_table_as_sql(True, temp_relation, sql)}} {%- endcall -%} {% do adapter.expand_target_column_types( from_relation=temp_relation, 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 4db19c2..fa9b58d 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 @@ -7,7 +7,7 @@ {% set contract_config = config.get('contract') %} - {{ create_view_as(tmp_relation, sql) }} + {{ get_create_view_as_sql(tmp_relation, sql) }} {% if contract_config.enforced %} CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] diff --git a/dbt/include/fabric/macros/materializations/models/view/view.sql b/dbt/include/fabric/macros/materializations/models/view/view.sql index 74011c2..de280a3 100644 --- a/dbt/include/fabric/macros/materializations/models/view/view.sql +++ b/dbt/include/fabric/macros/materializations/models/view/view.sql @@ -3,7 +3,7 @@ {%- set target_relation = this.incorporate(type='view') -%} {{log("Target Relation "~target_relation)}} - {%- set relation = get_relation_without_caching(this) %} + {%- set relation = fabric__get_relation_without_caching(this) %} {% set existing_relation = none %} {% if (relation|length == 1) %} {% set existing_relation = get_or_create_relation(relation[0][0], relation[0][2] , relation[0][1] , relation[0][3])[1] %} diff --git a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql index ca12361..8a5b0bd 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql @@ -192,7 +192,7 @@ {% set temp_relation = make_temp_relation(target_relation) %} {% set select = snapshot_staging_table(strategy, temp_snapshot_relation, target_relation) %} {% call statement('build_snapshot_staging_relation') %} - {{ create_table_as(True, temp_relation, select) }} + {{ get_create_table_as_sql(True, temp_relation, select) }} {% endcall %} {% do return(temp_relation) %} diff --git a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql index 039ae61..1d496e8 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql @@ -39,7 +39,7 @@ {% if not target_relation_exists %} {% set build_sql = build_snapshot_table(strategy, temp_snapshot_relation) %} - {% set final_sql = create_table_as(False, target_relation, build_sql) %} + {% set final_sql = get_create_table_as_sql(False, target_relation, build_sql) %} {% else %} From 2d34916a9f0b38e9cc2b15315b4b316dc7c05534 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Tue, 21 May 2024 23:40:10 -0700 Subject: [PATCH 03/17] Updating dbt-fabric adapter to stay compatible with dbt-synapse adapter --- .../fabric/macros/adapters/metadata.sql | 16 ++- .../fabric/macros/adapters/relation.sql | 6 +- dbt/include/fabric/macros/adapters/schema.sql | 2 +- .../models/incremental/incremental.sql | 9 +- .../materializations/models/table/clone.sql | 2 +- .../models/table/create_table_as.sql | 4 +- .../materializations/models/table/table.sql | 13 +- .../models/view/create_view_as.sql | 2 +- .../materializations/models/view/view.sql | 14 +-- tests/functional/adapter/test_caching.py | 115 ++---------------- tests/functional/adapter/test_constraints.py | 18 ++- .../test_list_relations_without_caching.py | 5 +- 12 files changed, 60 insertions(+), 146 deletions(-) diff --git a/dbt/include/fabric/macros/adapters/metadata.sql b/dbt/include/fabric/macros/adapters/metadata.sql index bfea1ef..4f4fa0a 100644 --- a/dbt/include/fabric/macros/adapters/metadata.sql +++ b/dbt/include/fabric/macros/adapters/metadata.sql @@ -9,6 +9,18 @@ information_schema {%- endmacro %} +{% macro get_use_database_sql(database) %} + {{ return(adapter.dispatch('get_use_database_sql', 'dbt')(database)) }} +{% endmacro %} + +{%- macro default__get_use_database_sql(database) -%} +{%- endmacro -%} + + +{%- macro fabric__get_use_database_sql(database) -%} + USE [{{database}}]; +{%- endmacro -%} + {% macro fabric__list_schemas(database) %} {% call statement('list_schemas', fetch_result=True, auto_begin=False) -%} select name as [schema] @@ -27,7 +39,7 @@ {% macro fabric__list_relations_without_caching(schema_relation) -%} {% call statement('list_relations_without_caching', fetch_result=True) -%} - USE [{{ schema_relation.database }}]; + {{ get_use_database_sql(schema_relation.database) }} with base as ( select DB_NAME() as [database], @@ -51,7 +63,7 @@ {% macro fabric__get_relation_without_caching(schema_relation) -%} {% call statement('get_relation_without_caching', fetch_result=True) -%} - USE [{{ schema_relation.database }}]; + {{ get_use_database_sql(schema_relation.database) }} with base as ( select DB_NAME() as [database], diff --git a/dbt/include/fabric/macros/adapters/relation.sql b/dbt/include/fabric/macros/adapters/relation.sql index b8f607b..677c847 100644 --- a/dbt/include/fabric/macros/adapters/relation.sql +++ b/dbt/include/fabric/macros/adapters/relation.sql @@ -16,7 +16,7 @@ {% if relation.type == 'view' -%} {% call statement('find_references', fetch_result=true) %} - USE [{{ relation.database }}]; + {{ get_use_database_sql(relation.database) }} select sch.name as schema_name, obj.name as view_name @@ -44,13 +44,13 @@ {%- else -%} {{ exceptions.raise_not_implemented('Invalid relation being dropped: ' ~ relation) }} {% endif %} - USE [{{ relation.database }}]; + {{ get_use_database_sql(relation.database) }} EXEC('DROP {{ relation.type }} IF EXISTS {{ relation.include(database=False) }};'); {% endmacro %} {% macro fabric__rename_relation(from_relation, to_relation) -%} {% call statement('rename_relation') -%} - USE [{{ from_relation.database }}]; + {{ get_use_database_sql(from_relation.database) }} EXEC sp_rename '{{ from_relation.schema }}.{{ from_relation.identifier }}', '{{ to_relation.identifier }}' {%- endcall %} {% endmacro %} diff --git a/dbt/include/fabric/macros/adapters/schema.sql b/dbt/include/fabric/macros/adapters/schema.sql index 7fa1083..099ba59 100644 --- a/dbt/include/fabric/macros/adapters/schema.sql +++ b/dbt/include/fabric/macros/adapters/schema.sql @@ -10,7 +10,7 @@ {% macro fabric__create_schema_with_authorization(relation, schema_authorization) -%} {% call statement('create_schema') -%} - USE [{{ relation.database }}]; + {{ get_use_database_sql(relation.database) }} IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ relation.schema }}') BEGIN EXEC('CREATE SCHEMA [{{ relation.schema }}] AUTHORIZATION [{{ schema_authorization }}]') diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index 83e8207..bac562c 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -3,8 +3,9 @@ {%- set full_refresh_mode = (should_full_refresh()) -%} {% set target_relation = this.incorporate(type='table') %} - {%- set relations_list = fabric__get_relation_without_caching(target_relation) -%} + {%- set existing_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} + {# {%- set relations_list = fabric__get_relation_without_caching(target_relation) -%} {%- set existing_relation = none %} {% if (relations_list|length == 1) and (relations_list[0][2] == target_relation.schema) and (relations_list[0][1] == target_relation.identifier) and (relations_list[0][3] == target_relation.type)%} @@ -12,7 +13,7 @@ {% elif (relations_list|length == 1) and (relations_list[0][2] == target_relation.schema) and (relations_list[0][1] == target_relation.identifier) and (relations_list[0][3] != target_relation.type) %} {% set existing_relation = get_or_create_relation(relations_list[0][0], relations_list[0][2] , relations_list[0][1] , relations_list[0][3])[1] %} - {% endif %} + {% endif %} #} {# {{ log("Full refresh mode" ~ full_refresh_mode)}} {{ log("existing relation : "~existing_relation ~ " type "~ existing_relation.type ~ " is view? "~existing_relation.is_view) }} @@ -38,7 +39,7 @@ {#-- Can't overwrite a view with a table - we must drop --#} {{ log("Dropping relation " ~ target_relation ~ " because it is a view and this model is a table.") }} - {{ drop_relation_if_exists(existing_relation) }} + {{ drop_relation(existing_relation) }} {%- call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql)}} {%- endcall -%} @@ -72,7 +73,7 @@ {%- endcall -%} {% endif %} - {% do drop_relation_if_exists(temp_relation) %} + {% do drop_relation(temp_relation) %} {{ run_hooks(post_hooks, inside_transaction=True) }} {% set target_relation = target_relation.incorporate(type='table') %} diff --git a/dbt/include/fabric/macros/materializations/models/table/clone.sql b/dbt/include/fabric/macros/materializations/models/table/clone.sql index b2e9d24..292c882 100644 --- a/dbt/include/fabric/macros/materializations/models/table/clone.sql +++ b/dbt/include/fabric/macros/materializations/models/table/clone.sql @@ -24,7 +24,7 @@ {%- set target_relation = this.incorporate(type='table') -%} {% call statement('main') %} - {{ drop_relation_if_exists(target_relation) }} + {{ drop_relation(target_relation) }} {{ create_or_replace_clone(target_relation, defer_relation) }} {% endcall %} {{ return({'relations': [target_relation]}) }} 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 fa9b58d..dcaadc5 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 @@ -3,7 +3,7 @@ {% set tmp_relation = relation.incorporate( path={"identifier": relation.identifier.replace("#", "") ~ '_temp_view'}, type='view')-%} - {% do run_query(drop_relation_if_exists(tmp_relation)) %} + {% do run_query(drop_relation(tmp_relation)) %} {% set contract_config = config.get('contract') %} @@ -27,6 +27,6 @@ EXEC('CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] AS (SELECT * FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]);'); {% endif %} - {{ drop_relation_if_exists(tmp_relation) }} + {{ drop_relation(tmp_relation) }} {% endmacro %} diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index 24f71f3..f9ee7fb 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -2,16 +2,9 @@ -- Load target relation {%- set target_relation = this.incorporate(type='table') %} - -- Load existing relation - {%- set relation = fabric__get_relation_without_caching(this) %} - - {% set existing_relation = none %} - {% if (relation|length == 1) %} - {% set existing_relation = get_or_create_relation(relation[0][0], relation[0][2] , relation[0][1] , relation[0][3])[1] %} - {% endif %} + {%- set existing_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} {%- set backup_relation = none %} - {{log("Existing Relation type is "~ existing_relation.type)}} {% if (existing_relation != none and existing_relation.type == "table") %} {%- set backup_relation = make_backup_relation(target_relation, 'table') -%} {% elif (existing_relation != none and existing_relation.type == "view") %} @@ -20,7 +13,7 @@ {% if (existing_relation != none) %} -- drop the temp relations if they exist already in the database - {{ drop_relation_if_exists(backup_relation) }} + {{ drop_relation(backup_relation) }} -- Rename target relation as backup relation {{ adapter.rename_relation(existing_relation, backup_relation) }} {% endif %} @@ -48,7 +41,7 @@ -- 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) }} + {{ drop_relation(backup_relation) }} {% endif %} -- Add constraints including FK relation. {{ fabric__build_model_constraints(target_relation) }} 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 cfb610f..f0ec12d 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 @@ -6,7 +6,7 @@ {%- set temp_view_sql = sql.replace("'", "''") -%} - USE [{{ relation.database }}]; + {{ get_use_database_sql(relation.database) }} {% set contract_config = config.get('contract') %} {% if contract_config.enforced %} {{ get_assert_columns_equivalent(sql) }} diff --git a/dbt/include/fabric/macros/materializations/models/view/view.sql b/dbt/include/fabric/macros/materializations/models/view/view.sql index de280a3..1c9041c 100644 --- a/dbt/include/fabric/macros/materializations/models/view/view.sql +++ b/dbt/include/fabric/macros/materializations/models/view/view.sql @@ -1,17 +1,9 @@ {% materialization view, adapter='fabric' -%} {%- set target_relation = this.incorporate(type='view') -%} - {{log("Target Relation "~target_relation)}} - - {%- set relation = fabric__get_relation_without_caching(this) %} - {% set existing_relation = none %} - {% if (relation|length == 1) %} - {% set existing_relation = get_or_create_relation(relation[0][0], relation[0][2] , relation[0][1] , relation[0][3])[1] %} - {% endif %} - {# {{log("Existing Relation "~existing_relation)}} #} + {%- set existing_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} {%- set backup_relation = none %} - {# {{log("Existing Relation type is "~ existing_relation.type)}} #} {% if (existing_relation != none and existing_relation.type == "table") %} {%- set backup_relation = make_backup_relation(target_relation, 'table') -%} {% elif (existing_relation != none and existing_relation.type == "view") %} @@ -20,7 +12,7 @@ {% if (existing_relation != none) %} -- drop the temp relations if they exist already in the database - {{ drop_relation_if_exists(backup_relation) }} + {{ drop_relation(backup_relation) }} -- Rename target relation as backup relation {{ adapter.rename_relation(existing_relation, backup_relation) }} {% endif %} @@ -43,7 +35,7 @@ {{ run_hooks(post_hooks, inside_transaction=True) }} {{ adapter.commit() }} {% if (backup_relation != none) %} - {{ drop_relation_if_exists(backup_relation) }} + {{ drop_relation(backup_relation) }} {% endif %} {{ run_hooks(post_hooks, inside_transaction=False) }} {{ return({'relations': [target_relation]}) }} diff --git a/tests/functional/adapter/test_caching.py b/tests/functional/adapter/test_caching.py index 11a9c4d..071e4e1 100644 --- a/tests/functional/adapter/test_caching.py +++ b/tests/functional/adapter/test_caching.py @@ -1,117 +1,22 @@ -import pytest -from dbt.tests.util import run_dbt - -model_sql = """ -{{ - config( - materialized='table' - ) -}} -select 1 as id -""" - -another_schema_model_sql = """ -{{ - config( - materialized='table', - schema='another_schema' - ) -}} -select 1 as id -""" - - -class BaseCachingTest: - @pytest.fixture(scope="class") - def project_config_update(self): - return { - "config-version": 2, - "quoting": { - "identifier": False, - "schema": False, - }, - } - - def run_and_inspect_cache(self, project, run_args=None): - run_dbt(run_args) - - # the cache was empty at the start of the run. - # the model materialization returned an unquoted relation and added to the cache. - adapter = project.adapter - assert len(adapter.cache.relations) == 1 - relation = list(adapter.cache.relations).pop() - assert relation.schema == project.test_schema - assert relation.schema == project.test_schema.lower() - - # on the second run, dbt will find a relation in the database during cache population. - # this relation will be quoted, because list_relations_without_caching (by default) uses - # quote_policy = {"database": True, "schema": True, "identifier": True} - # when adding relations to the cache. - run_dbt(run_args) - adapter = project.adapter - assert len(adapter.cache.relations) == 1 - second_relation = list(adapter.cache.relations).pop() - - # perform a case-insensitive + quote-insensitive comparison - for key in ["database", "schema", "identifier"]: - assert getattr(relation, key).lower() == getattr(second_relation, key).lower() - - def test_cache(self, project): - self.run_and_inspect_cache(project, run_args=["run"]) - - -class BaseCachingLowercaseModel(BaseCachingTest): - @pytest.fixture(scope="class") - def models(self): - return { - "model.sql": model_sql, - } - - -class BaseCachingUppercaseModel(BaseCachingTest): - @pytest.fixture(scope="class") - def models(self): - return { - "MODEL.sql": model_sql, - } - - -class BaseCachingSelectedSchemaOnly(BaseCachingTest): - @pytest.fixture(scope="class") - def models(self): - return { - "model.sql": model_sql, - "another_schema_model.sql": another_schema_model_sql, - } - - def test_cache(self, project): - # this should only cache the schema containing the selected model - run_args = ["--cache-selected-only", "run", "--select", "model"] - self.run_and_inspect_cache(project, run_args) - - -class TestNoPopulateCache(BaseCachingTest): - @pytest.fixture(scope="class") - def models(self): - return { - "model.sql": model_sql, - } - - def test_cache(self, project): - # --no-populate-cache still allows the cache to populate all relations - # under a schema, so the behavior here remains the same as other tests - run_args = ["--no-populate-cache", "run"] - self.run_and_inspect_cache(project, run_args) +from dbt.tests.adapter.caching.test_caching import ( + BaseCachingLowercaseModel, + BaseCachingSelectedSchemaOnly, + BaseCachingUppercaseModel, + BaseNoPopulateCache, +) class TestCachingLowerCaseModel(BaseCachingLowercaseModel): pass -@pytest.mark.skip(reason="Fabric DW does not support Case Insensivity.") class TestCachingUppercaseModel(BaseCachingUppercaseModel): pass class TestCachingSelectedSchemaOnly(BaseCachingSelectedSchemaOnly): pass + + +class TestNoPopulateCache(BaseNoPopulateCache): + pass diff --git a/tests/functional/adapter/test_constraints.py b/tests/functional/adapter/test_constraints.py index 6ff5265..8ea6ce6 100644 --- a/tests/functional/adapter/test_constraints.py +++ b/tests/functional/adapter/test_constraints.py @@ -480,9 +480,11 @@ 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) ) 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): unformatted_constraint_schema_yml = read_file("models", "constraints_schema.yml") write_file( @@ -501,7 +503,9 @@ def test__constraints_ddl(self, project, expected_sql): generated_sql_generic, "foreign_key_model", "" ) generated_sql_wodb = generated_sql_generic.replace("USE [" + project.database + "];", "") - assert _normalize_whitespace(expected_sql) == _normalize_whitespace(generated_sql_wodb) + assert _normalize_whitespace(expected_sql.strip()) == _normalize_whitespace( + generated_sql_wodb.strip() + ) class TestTableConstraintsRuntimeDdlEnforcement(BaseConstraintsRuntimeDdlEnforcement): @@ -542,9 +546,11 @@ 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) ) 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): unformatted_constraint_schema_yml = read_file("models", "constraints_schema.yml") write_file( @@ -562,7 +568,11 @@ def test__model_constraints_ddl(self, project, expected_sql): generated_sql_generic, "foreign_key_model", "" ) generated_sql_wodb = generated_sql_generic.replace("USE [" + project.database + "];", "") - assert _normalize_whitespace(expected_sql) == _normalize_whitespace(generated_sql_wodb) + print("Expected:", expected_sql.strip()) + print("Generated:", generated_sql_wodb.strip()) + assert _normalize_whitespace(expected_sql.strip()) == _normalize_whitespace( + generated_sql_wodb.strip() + ) class TestModelConstraintsRuntimeEnforcement(BaseModelConstraintsRuntimeEnforcement): diff --git a/tests/functional/adapter/test_list_relations_without_caching.py b/tests/functional/adapter/test_list_relations_without_caching.py index 3c43761..f581fa9 100644 --- a/tests/functional/adapter/test_list_relations_without_caching.py +++ b/tests/functional/adapter/test_list_relations_without_caching.py @@ -127,7 +127,8 @@ def test__fabric__list_relations_without_caching(self, project): # n_relations = find_result_in_parsed_logs(parsed_logs, "n_relations") # assert n_relations == "n_relations: 1" - assert "n_relations: 1" in log_output + # assert "n_relations: 1" in log_output + assert "n_relations: 2" in log_output class TestListRelationsWithoutCachingFull: @@ -170,4 +171,4 @@ def test__fabric__list_relations_without_caching(self, project): # n_relations = find_result_in_parsed_logs(parsed_logs, "n_relations") # assert n_relations == f"n_relations: {NUM_EXPECTED_RELATIONS}" - assert f"n_relations: {NUM_EXPECTED_RELATIONS}" in log_output + assert "n_relations: 12" in log_output From 36187bf956625f31877c8fdaf7aea4cc3a3b22fd Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Wed, 22 May 2024 23:08:07 -0700 Subject: [PATCH 04/17] changes to drop_relation_if_exists --- .../fabric/macros/adapters/relation.sql | 52 ++++++++----------- dbt/include/fabric/macros/adapters/schema.sql | 2 +- .../models/incremental/incremental.sql | 20 ++++--- .../materializations/models/table/clone.sql | 2 +- .../materializations/models/table/table.sql | 4 +- .../materializations/models/view/view.sql | 4 +- .../materializations/snapshots/helpers.sql | 2 +- tests/functional/adapter/test_caching.py | 2 + 8 files changed, 45 insertions(+), 43 deletions(-) diff --git a/dbt/include/fabric/macros/adapters/relation.sql b/dbt/include/fabric/macros/adapters/relation.sql index 677c847..4899c95 100644 --- a/dbt/include/fabric/macros/adapters/relation.sql +++ b/dbt/include/fabric/macros/adapters/relation.sql @@ -7,45 +7,39 @@ {% endmacro %} {% macro fabric__drop_relation(relation) -%} - {% call statement('drop_relation', auto_begin=False) -%} - {{ fabric__drop_relation_script(relation) }} - {%- endcall %} -{% endmacro %} - -{% macro fabric__drop_relation_script(relation) -%} - - {% if relation.type == 'view' -%} + {% if relation.type == 'view' -%} {% call statement('find_references', fetch_result=true) %} - {{ get_use_database_sql(relation.database) }} - select - sch.name as schema_name, - obj.name as view_name - from sys.sql_expression_dependencies refs - inner join sys.objects obj - on refs.referencing_id = obj.object_id - inner join sys.schemas sch - on obj.schema_id = sch.schema_id - where refs.referenced_database_name = '{{ relation.database }}' - and refs.referenced_schema_name = '{{ relation.schema }}' - and refs.referenced_entity_name = '{{ relation.identifier }}' - and refs.referencing_class = 1 - and obj.type = 'V' + {{ get_use_database_sql(relation.database) }} + select + sch.name as schema_name, + obj.name as view_name + from sys.sql_expression_dependencies refs + inner join sys.objects obj + on refs.referencing_id = obj.object_id + inner join sys.schemas sch + on obj.schema_id = sch.schema_id + where refs.referenced_database_name = '{{ relation.database }}' + and refs.referenced_schema_name = '{{ relation.schema }}' + and refs.referenced_entity_name = '{{ relation.identifier }}' + and refs.referencing_class = 1 + and obj.type = 'V' {% endcall %} {% set references = load_result('find_references')['data'] %} {% for reference in references -%} - -- dropping referenced view {{ reference[0] }}.{{ reference[1] }} - {{ fabric__drop_relation_script(relation.incorporate( - type="view", - path={"schema": reference[0], "identifier": reference[1]})) }} + -- dropping referenced view {{ reference[0] }}.{{ reference[1] }} + {{ drop_relation_if_exists(relation.incorporate( + type="view", + path={"schema": reference[0], "identifier": reference[1]})) }} {% endfor %} + {{ get_use_database_sql(relation.database) }} + EXEC('DROP VIEW IF EXISTS {{ relation.include(database=False) }};'); {% elif relation.type == 'table'%} {% set object_id_type = 'U' %} - + {{ get_use_database_sql(relation.database) }} + EXEC('DROP TABLE IF EXISTS {{ relation.include(database=False) }};'); {%- else -%} {{ exceptions.raise_not_implemented('Invalid relation being dropped: ' ~ relation) }} {% endif %} - {{ get_use_database_sql(relation.database) }} - EXEC('DROP {{ relation.type }} IF EXISTS {{ relation.include(database=False) }};'); {% endmacro %} {% macro fabric__rename_relation(from_relation, to_relation) -%} diff --git a/dbt/include/fabric/macros/adapters/schema.sql b/dbt/include/fabric/macros/adapters/schema.sql index 099ba59..f85c056 100644 --- a/dbt/include/fabric/macros/adapters/schema.sql +++ b/dbt/include/fabric/macros/adapters/schema.sql @@ -27,7 +27,7 @@ identifier=row[1], type=row[3] ) -%} - {% do drop_relation(schema_relation) %} + {% do drop_relation_if_exists(schema_relation) %} {%- endfor %} {% call statement('drop_schema') -%} diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index bac562c..49547ff 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -3,8 +3,14 @@ {%- set full_refresh_mode = (should_full_refresh()) -%} {% set target_relation = this.incorporate(type='table') %} - {%- set existing_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} + {%- set relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} + {%- set existing_relation = none %} + {% if (relation.type == target_relation.type) and (relation.identifier == target_relation.identifier) and (relation.schema == target_relation.schema) and (relation.database == target_relation.database) %} + {% set existing_relation = target_relation %} + {% elif (relation.type != target_relation.type) and (relation.identifier == target_relation.identifier) and (relation.schema == target_relation.schema) and (relation.database == target_relation.database) %} + {% set existing_relation = get_or_create_relation(relation.database, relation.schema, relation.identifier, relation.type)[1] %} + {% endif %} {# {%- set relations_list = fabric__get_relation_without_caching(target_relation) -%} {%- set existing_relation = none %} {% if (relations_list|length == 1) and (relations_list[0][2] == target_relation.schema) @@ -19,6 +25,9 @@ {{ log("existing relation : "~existing_relation ~ " type "~ existing_relation.type ~ " is view? "~existing_relation.is_view) }} {{ log("target relation: " ~target_relation ~ " type "~ target_relation.type ~ " is view? "~target_relation.is_view) }} #} + {{ log("existing relation : "~existing_relation ~ " type "~ existing_relation.type ~ " is view? "~existing_relation.is_view) }} + {{ log("target relation: " ~target_relation ~ " type "~ target_relation.type ~ " is view? "~target_relation.is_view) }} + -- configs {%- set unique_key = config.get('unique_key') -%} {% set incremental_strategy = config.get('incremental_strategy') or 'default' %} @@ -30,28 +39,25 @@ {{ run_hooks(pre_hooks, inside_transaction=True) }} {% if existing_relation is none %} - {%- call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql)}} {%- endcall -%} {% elif existing_relation.is_view %} - {#-- Can't overwrite a view with a table - we must drop --#} {{ log("Dropping relation " ~ target_relation ~ " because it is a view and this model is a table.") }} - {{ drop_relation(existing_relation) }} + {{ drop_relation_if_exists(existing_relation) }} + {%- call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql)}} {%- endcall -%} {% elif full_refresh_mode %} - {%- call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql)}} {%- endcall -%} {% else %} - {%- call statement('create_tmp_relation') -%} {{ get_create_table_as_sql(True, temp_relation, sql)}} {%- endcall -%} @@ -73,7 +79,7 @@ {%- endcall -%} {% endif %} - {% do drop_relation(temp_relation) %} + {% do drop_relation_if_exists(temp_relation) %} {{ run_hooks(post_hooks, inside_transaction=True) }} {% set target_relation = target_relation.incorporate(type='table') %} diff --git a/dbt/include/fabric/macros/materializations/models/table/clone.sql b/dbt/include/fabric/macros/materializations/models/table/clone.sql index 292c882..b2e9d24 100644 --- a/dbt/include/fabric/macros/materializations/models/table/clone.sql +++ b/dbt/include/fabric/macros/materializations/models/table/clone.sql @@ -24,7 +24,7 @@ {%- set target_relation = this.incorporate(type='table') -%} {% call statement('main') %} - {{ drop_relation(target_relation) }} + {{ drop_relation_if_exists(target_relation) }} {{ create_or_replace_clone(target_relation, defer_relation) }} {% endcall %} {{ return({'relations': [target_relation]}) }} diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index f9ee7fb..2977dc3 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -13,7 +13,7 @@ {% if (existing_relation != none) %} -- drop the temp relations if they exist already in the database - {{ drop_relation(backup_relation) }} + {{ drop_relation_if_exists(backup_relation) }} -- Rename target relation as backup relation {{ adapter.rename_relation(existing_relation, backup_relation) }} {% endif %} @@ -41,7 +41,7 @@ -- finally, drop the foreign key references if exists {{ drop_fk_indexes_on_table(backup_relation) }} -- drop existing/backup relation after the commit - {{ drop_relation(backup_relation) }} + {{ drop_relation_if_exists(backup_relation) }} {% endif %} -- Add constraints including FK relation. {{ fabric__build_model_constraints(target_relation) }} diff --git a/dbt/include/fabric/macros/materializations/models/view/view.sql b/dbt/include/fabric/macros/materializations/models/view/view.sql index 1c9041c..40f7752 100644 --- a/dbt/include/fabric/macros/materializations/models/view/view.sql +++ b/dbt/include/fabric/macros/materializations/models/view/view.sql @@ -12,7 +12,7 @@ {% if (existing_relation != none) %} -- drop the temp relations if they exist already in the database - {{ drop_relation(backup_relation) }} + {{ drop_relation_if_exists(backup_relation) }} -- Rename target relation as backup relation {{ adapter.rename_relation(existing_relation, backup_relation) }} {% endif %} @@ -35,7 +35,7 @@ {{ run_hooks(post_hooks, inside_transaction=True) }} {{ adapter.commit() }} {% if (backup_relation != none) %} - {{ drop_relation(backup_relation) }} + {{ drop_relation_if_exists(backup_relation) }} {% endif %} {{ run_hooks(post_hooks, inside_transaction=False) }} {{ return({'relations': [target_relation]}) }} diff --git a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql index 8a5b0bd..a37ae64 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/helpers.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/helpers.sql @@ -1,6 +1,6 @@ {% macro fabric__post_snapshot(staging_relation) %} -- Clean up the snapshot temp table - {% do drop_relation(staging_relation) %} + {% do drop_relation_if_exists(staging_relation) %} {% endmacro %} --Due to Alter not being supported, have to rely on this for temporarily diff --git a/tests/functional/adapter/test_caching.py b/tests/functional/adapter/test_caching.py index 071e4e1..186ae69 100644 --- a/tests/functional/adapter/test_caching.py +++ b/tests/functional/adapter/test_caching.py @@ -1,3 +1,4 @@ +import pytest from dbt.tests.adapter.caching.test_caching import ( BaseCachingLowercaseModel, BaseCachingSelectedSchemaOnly, @@ -10,6 +11,7 @@ class TestCachingLowerCaseModel(BaseCachingLowercaseModel): pass +@pytest.mark.skip(reason="Fabric DW does not support Case Insensivity.") class TestCachingUppercaseModel(BaseCachingUppercaseModel): pass From 35110fe6fed26a6a5f74e2a11ad6a7dcd5377133 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Thu, 23 May 2024 02:38:58 -0700 Subject: [PATCH 05/17] Updated tests and drop_relation --- dbt/include/fabric/macros/adapters/columns.sql | 4 ++-- dbt/include/fabric/macros/adapters/relation.sql | 12 +++++------- dbt/include/fabric/macros/adapters/schema.sql | 2 +- .../models/incremental/incremental.sql | 4 ++-- .../macros/materializations/models/table/clone.sql | 2 +- .../models/table/create_table_as.sql | 5 ++--- .../macros/materializations/models/table/table.sql | 5 +++-- .../macros/materializations/models/view/view.sql | 4 ++-- .../macros/materializations/snapshots/snapshot.sql | 2 +- tests/functional/adapter/test_aliases.py | 3 --- tests/functional/adapter/test_basic.py | 6 +----- .../adapter/test_changing_relation_type.py | 2 -- tests/functional/adapter/test_empty.py | 1 - tests/functional/adapter/test_incremental.py | 7 +++---- .../adapter/test_list_relations_without_caching.py | 14 -------------- tests/functional/adapter/test_query_comment.py | 1 - .../functional/adapter/test_store_test_failures.py | 1 - 17 files changed, 23 insertions(+), 52 deletions(-) diff --git a/dbt/include/fabric/macros/adapters/columns.sql b/dbt/include/fabric/macros/adapters/columns.sql index db081ff..522cc9c 100644 --- a/dbt/include/fabric/macros/adapters/columns.sql +++ b/dbt/include/fabric/macros/adapters/columns.sql @@ -54,8 +54,8 @@ {% macro fabric__alter_column_type(relation, column_name, new_column_type) %} - {%- set table_name= tmp_relation.include(database=False).include(schema=False)-%} - {%- set schema_name = tmp_relation.include(database=False).include(identifier=False) -%} + {%- set table_name= relation.include(database=False).include(schema=False)-%} + {%- set schema_name = relation.include(database=False).include(identifier=False) -%} {% set generate_tmp_relation_script %} SELECT TRIM(REPLACE(STRING_AGG(ColumnName + ' ', ',-'), '-', CHAR(10))) AS ColumnDef diff --git a/dbt/include/fabric/macros/adapters/relation.sql b/dbt/include/fabric/macros/adapters/relation.sql index 4899c95..3fa4ff8 100644 --- a/dbt/include/fabric/macros/adapters/relation.sql +++ b/dbt/include/fabric/macros/adapters/relation.sql @@ -6,7 +6,7 @@ {{ return(temp_relation) }} {% endmacro %} -{% macro fabric__drop_relation(relation) -%} +{% macro fabric__get_drop_sql(relation) -%} {% if relation.type == 'view' -%} {% call statement('find_references', fetch_result=true) %} {{ get_use_database_sql(relation.database) }} @@ -27,19 +27,17 @@ {% set references = load_result('find_references')['data'] %} {% for reference in references -%} -- dropping referenced view {{ reference[0] }}.{{ reference[1] }} - {{ drop_relation_if_exists(relation.incorporate( + {% do adapter.drop_relation(relation.incorporate( type="view", - path={"schema": reference[0], "identifier": reference[1]})) }} + path={"schema": reference[0], "identifier": reference[1]})) %} {% endfor %} - {{ get_use_database_sql(relation.database) }} - EXEC('DROP VIEW IF EXISTS {{ relation.include(database=False) }};'); {% elif relation.type == 'table'%} {% set object_id_type = 'U' %} - {{ get_use_database_sql(relation.database) }} - EXEC('DROP TABLE IF EXISTS {{ relation.include(database=False) }};'); {%- else -%} {{ exceptions.raise_not_implemented('Invalid relation being dropped: ' ~ relation) }} {% endif %} + {{ get_use_database_sql(relation.database) }} + EXEC('DROP {{ relation.type }} IF EXISTS {{ relation.include(database=False) }};'); {% endmacro %} {% macro fabric__rename_relation(from_relation, to_relation) -%} diff --git a/dbt/include/fabric/macros/adapters/schema.sql b/dbt/include/fabric/macros/adapters/schema.sql index f85c056..a117e47 100644 --- a/dbt/include/fabric/macros/adapters/schema.sql +++ b/dbt/include/fabric/macros/adapters/schema.sql @@ -27,7 +27,7 @@ identifier=row[1], type=row[3] ) -%} - {% do drop_relation_if_exists(schema_relation) %} + {% do adapter.drop_relation(schema_relation) %} {%- endfor %} {% call statement('drop_schema') -%} diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index 49547ff..698b213 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -46,7 +46,7 @@ {% elif existing_relation.is_view %} {#-- Can't overwrite a view with a table - we must drop --#} {{ log("Dropping relation " ~ target_relation ~ " because it is a view and this model is a table.") }} - {{ drop_relation_if_exists(existing_relation) }} + {% do adapter.drop_relation(existing_relation) %} {%- call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql)}} @@ -79,7 +79,7 @@ {%- endcall -%} {% endif %} - {% do drop_relation_if_exists(temp_relation) %} + {% do adapter.drop_relation(temp_relation) %} {{ run_hooks(post_hooks, inside_transaction=True) }} {% set target_relation = target_relation.incorporate(type='table') %} diff --git a/dbt/include/fabric/macros/materializations/models/table/clone.sql b/dbt/include/fabric/macros/materializations/models/table/clone.sql index b2e9d24..b552e45 100644 --- a/dbt/include/fabric/macros/materializations/models/table/clone.sql +++ b/dbt/include/fabric/macros/materializations/models/table/clone.sql @@ -24,7 +24,7 @@ {%- set target_relation = this.incorporate(type='table') -%} {% call statement('main') %} - {{ drop_relation_if_exists(target_relation) }} + {% do adapter.drop_relation(target_relation) %} {{ create_or_replace_clone(target_relation, defer_relation) }} {% endcall %} {{ return({'relations': [target_relation]}) }} 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 dcaadc5..7c281f7 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 @@ -3,8 +3,7 @@ {% set tmp_relation = relation.incorporate( path={"identifier": relation.identifier.replace("#", "") ~ '_temp_view'}, type='view')-%} - {% do run_query(drop_relation(tmp_relation)) %} - + {% do adapter.drop_relation(tmp_relation) %} {% set contract_config = config.get('contract') %} {{ get_create_view_as_sql(tmp_relation, sql) }} @@ -27,6 +26,6 @@ EXEC('CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] AS (SELECT * FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]);'); {% endif %} - {{ drop_relation(tmp_relation) }} + {% do adapter.drop_relation(tmp_relation) %} {% endmacro %} diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index 2977dc3..84aefd6 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -13,7 +13,8 @@ {% if (existing_relation != none) %} -- drop the temp relations if they exist already in the database - {{ drop_relation_if_exists(backup_relation) }} + + {% do adapter.drop_relation(backup_relation) %} -- Rename target relation as backup relation {{ adapter.rename_relation(existing_relation, backup_relation) }} {% endif %} @@ -41,7 +42,7 @@ -- 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) }} + {% do adapter.drop_relation(backup_relation) %} {% endif %} -- Add constraints including FK relation. {{ fabric__build_model_constraints(target_relation) }} diff --git a/dbt/include/fabric/macros/materializations/models/view/view.sql b/dbt/include/fabric/macros/materializations/models/view/view.sql index 40f7752..e4aa8dc 100644 --- a/dbt/include/fabric/macros/materializations/models/view/view.sql +++ b/dbt/include/fabric/macros/materializations/models/view/view.sql @@ -12,7 +12,7 @@ {% if (existing_relation != none) %} -- drop the temp relations if they exist already in the database - {{ drop_relation_if_exists(backup_relation) }} + {% do adapter.drop_relation(backup_relation) %} -- Rename target relation as backup relation {{ adapter.rename_relation(existing_relation, backup_relation) }} {% endif %} @@ -35,7 +35,7 @@ {{ run_hooks(post_hooks, inside_transaction=True) }} {{ adapter.commit() }} {% if (backup_relation != none) %} - {{ drop_relation_if_exists(backup_relation) }} + {% do adapter.drop_relation(backup_relation) %} {% endif %} {{ run_hooks(post_hooks, inside_transaction=False) }} {{ return({'relations': [target_relation]}) }} diff --git a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql index 1d496e8..286f065 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql @@ -81,7 +81,7 @@ {{ final_sql }} {% endcall %} - {{ drop_relation_if_exists(temp_snapshot_relation) }} + {% do adapter.drop_relation(temp_snapshot_relation) %} {% set should_revoke = should_revoke(target_relation_exists, full_refresh_mode=False) %} {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} diff --git a/tests/functional/adapter/test_aliases.py b/tests/functional/adapter/test_aliases.py index f5d43b0..b426258 100644 --- a/tests/functional/adapter/test_aliases.py +++ b/tests/functional/adapter/test_aliases.py @@ -8,7 +8,6 @@ ) -@pytest.mark.skip(reason="CTAS is not supported without a table.") class TestAliasesFabric(BaseAliases): @pytest.fixture(scope="class") def macros(self): @@ -21,14 +20,12 @@ def macros(self): return {"expect_value.sql": MACROS__EXPECT_VALUE_SQL} -@pytest.mark.skip(reason="Test audit tables are using CTAS on View without a table definition.") class TestSameAliasDifferentSchemasFabric(BaseSameAliasDifferentSchemas): @pytest.fixture(scope="class") def macros(self): return {"expect_value.sql": MACROS__EXPECT_VALUE_SQL} -@pytest.mark.skip(reason="Test audit tables are using CTAS on View without a table definition.") class TestSameAliasDifferentDatabasesFabric(BaseSameAliasDifferentDatabases): @pytest.fixture(scope="class") def macros(self): diff --git a/tests/functional/adapter/test_basic.py b/tests/functional/adapter/test_basic.py index 65a1b49..ad833ca 100644 --- a/tests/functional/adapter/test_basic.py +++ b/tests/functional/adapter/test_basic.py @@ -19,12 +19,11 @@ class TestSimpleMaterializations(BaseSimpleMaterializations): pass -@pytest.mark.skip(reason="CTAS is not supported without a table.") class TestSingularTestsFabric(BaseSingularTests): pass -@pytest.mark.skip(reason="ephemeral not supported") +@pytest.mark.skip(reason="Nested CTE is not supported") class TestSingularTestsEphemeralFabric(BaseSingularTestsEphemeral): pass @@ -41,8 +40,6 @@ class TestIncrementalFabric(BaseIncremental): pass -# Modified incremental_not_schema_change.sql file to handle DATETIME compatibility issues. -@pytest.mark.skip(reason="CTAS is not supported without a table.") class TestIncrementalNotSchemaChangeFabric(BaseIncrementalNotSchemaChange): @pytest.fixture(scope="class") def models(self): @@ -73,7 +70,6 @@ class TestSnapshotTimestampFabric(BaseSnapshotTimestamp): pass -# Assertion Failed. class TestBaseCachingFabric(BaseAdapterMethod): pass diff --git a/tests/functional/adapter/test_changing_relation_type.py b/tests/functional/adapter/test_changing_relation_type.py index b0b6bcd..82984fa 100644 --- a/tests/functional/adapter/test_changing_relation_type.py +++ b/tests/functional/adapter/test_changing_relation_type.py @@ -1,7 +1,5 @@ -import pytest from dbt.tests.adapter.relations.test_changing_relation_type import BaseChangeRelationTypeValidator -@pytest.mark.skip(reason="CTAS is not supported without a underlying table definition.") class TestChangeRelationTypesFabric(BaseChangeRelationTypeValidator): pass diff --git a/tests/functional/adapter/test_empty.py b/tests/functional/adapter/test_empty.py index 10906a1..8af58cb 100644 --- a/tests/functional/adapter/test_empty.py +++ b/tests/functional/adapter/test_empty.py @@ -1,5 +1,4 @@ # from dbt.tests.adapter.empty.test_empty import BaseTestEmpty - # class TestEmpty(BaseTestEmpty): # pass diff --git a/tests/functional/adapter/test_incremental.py b/tests/functional/adapter/test_incremental.py index da1676a..a572745 100644 --- a/tests/functional/adapter/test_incremental.py +++ b/tests/functional/adapter/test_incremental.py @@ -87,12 +87,13 @@ """ -@pytest.mark.skip(reason="CTAS is not supported without a table.") class TestBaseIncrementalUniqueKeyFabric(BaseIncrementalUniqueKey): pass -@pytest.mark.skip(reason="CTAS is not supported without a table.") +@pytest.mark.skip( + "The specified ALTER TABLE statement is not supported in this edition of SQL Server." +) class TestIncrementalOnSchemaChangeFabric(BaseIncrementalOnSchemaChange): @pytest.fixture(scope="class") def models(self): @@ -112,12 +113,10 @@ def models(self): } -@pytest.mark.skip(reason="CTAS is not supported without a table.") class TestIncrementalPredicatesDeleteInsertFabric(BaseIncrementalPredicates): pass -@pytest.mark.skip(reason="CTAS is not supported without a table.") class TestPredicatesDeleteInsertFabric(BaseIncrementalPredicates): @pytest.fixture(scope="class") def project_config_update(self): diff --git a/tests/functional/adapter/test_list_relations_without_caching.py b/tests/functional/adapter/test_list_relations_without_caching.py index f581fa9..d728be7 100644 --- a/tests/functional/adapter/test_list_relations_without_caching.py +++ b/tests/functional/adapter/test_list_relations_without_caching.py @@ -108,8 +108,6 @@ def test__fabric__list_relations_without_caching(self, project): schemas = project.created_schemas for schema in schemas: - # schema_relation = BaseRelation.create(schema=schema, database=database) - # schema_relation = f"{database}.{schema}" kwargs = {"schema_relation": schema} _, log_output = run_dbt_and_capture( [ @@ -121,13 +119,6 @@ def test__fabric__list_relations_without_caching(self, project): str(kwargs), ] ) - - # parsed_logs = parse_json_logs(log_output) - # print(parsed_logs) - # n_relations = find_result_in_parsed_logs(parsed_logs, "n_relations") - - # assert n_relations == "n_relations: 1" - # assert "n_relations: 1" in log_output assert "n_relations: 2" in log_output @@ -166,9 +157,4 @@ def test__fabric__list_relations_without_caching(self, project): str(kwargs), ] ) - - # parsed_logs = parse_json_logs(log_output) - # n_relations = find_result_in_parsed_logs(parsed_logs, "n_relations") - - # assert n_relations == f"n_relations: {NUM_EXPECTED_RELATIONS}" assert "n_relations: 12" in log_output diff --git a/tests/functional/adapter/test_query_comment.py b/tests/functional/adapter/test_query_comment.py index 7e4781a..98fc95b 100644 --- a/tests/functional/adapter/test_query_comment.py +++ b/tests/functional/adapter/test_query_comment.py @@ -10,7 +10,6 @@ ) -# Tests # class TestQueryComments(BaseQueryComments): pass diff --git a/tests/functional/adapter/test_store_test_failures.py b/tests/functional/adapter/test_store_test_failures.py index 1439ee5..e079e3f 100644 --- a/tests/functional/adapter/test_store_test_failures.py +++ b/tests/functional/adapter/test_store_test_failures.py @@ -18,7 +18,6 @@ # TestStoreTestFailures, # ) - tests__passing_test = """ select * from {{ ref('fine_model') }} where 1=2 From 8cc922781c63c928294d26c47c330192ee9181ee Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 09:36:56 -0700 Subject: [PATCH 06/17] adding temp relation to table materialization to support temp view drop relation --- .../fabric/macros/materializations/models/table/table.sql | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index 84aefd6..d94e0ff 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -26,11 +26,19 @@ -- `BEGIN` happens here: {{ run_hooks(pre_hooks, inside_transaction=True) }} + -- creating a temp relation + {% set tmp_relation = target_relation.incorporate( + path={"identifier": target_relation.identifier.replace("#", "") ~ '_temp_view'}, + type='view')-%} + -- build model {% call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql) }} {%- endcall %} + -- drop temp relation + {% do adapter.drop_relation(tmp_relation) %} + -- cleanup {{ run_hooks(post_hooks, inside_transaction=True) }} {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} From 1e72de21c4b96420ddf717f7bcd0dfc87dd78780 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 09:47:05 -0700 Subject: [PATCH 07/17] adding a log --- .../fabric/macros/materializations/models/table/table.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index d94e0ff..37727fd 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -37,6 +37,7 @@ {%- endcall %} -- drop temp relation + {{log ("I am here")}} {% do adapter.drop_relation(tmp_relation) %} -- cleanup From 4aa58071196c27ed2cc6f814458fbb9c3df75166 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 10:38:21 -0700 Subject: [PATCH 08/17] dropping and creating temp relation in table materialization and removing it from adapter create_table_as.sql macro --- .../materializations/models/table/table.sql | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index 37727fd..ebe9530 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -12,8 +12,8 @@ {% endif %} {% if (existing_relation != none) %} - -- drop the temp relations if they exist already in the database + -- drop the temp relations if they exist already in the database {% do adapter.drop_relation(backup_relation) %} -- Rename target relation as backup relation {{ adapter.rename_relation(existing_relation, backup_relation) }} @@ -26,22 +26,24 @@ -- `BEGIN` happens here: {{ run_hooks(pre_hooks, inside_transaction=True) }} - -- creating a temp relation - {% set tmp_relation = target_relation.incorporate( - path={"identifier": target_relation.identifier.replace("#", "") ~ '_temp_view'}, - type='view')-%} + -- naming a temp relation + {% set tmp_relation = target_relation.incorporate(path={"identifier": target_relation.identifier ~ '__dbt_tmp_vw'}, type='view')-%} + + -- Fabric & Synapse adapters use temp relation because of lack of CTE support for CTE in CTAS, Insert + -- drop temp relation if exists + {% do adapter.drop_relation(tmp_relation) %} -- build model {% call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql) }} {%- endcall %} - -- drop temp relation - {{log ("I am here")}} + -- drop temp relation if exists {% do adapter.drop_relation(tmp_relation) %} -- 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 @@ -53,6 +55,7 @@ -- drop existing/backup relation after the commit {% do adapter.drop_relation(backup_relation) %} {% endif %} + -- Add constraints including FK relation. {{ fabric__build_model_constraints(target_relation) }} {{ run_hooks(post_hooks, inside_transaction=False) }} From 916d48d016a66a8f01eb39106109a5f7355d965b Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 11:26:35 -0700 Subject: [PATCH 09/17] removing hardcoded fabric__ references to not to break dbt-synapse and dbt-sqlserver adapters --- .../models/incremental/incremental.sql | 20 ++----------------- .../models/table/columns_spec_ddl.sql | 14 +++++++++++++ .../models/table/create_table_as.sql | 17 ++++++---------- .../materializations/models/table/table.sql | 2 +- 4 files changed, 23 insertions(+), 30 deletions(-) diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index 698b213..5fce98c 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -11,22 +11,6 @@ {% elif (relation.type != target_relation.type) and (relation.identifier == target_relation.identifier) and (relation.schema == target_relation.schema) and (relation.database == target_relation.database) %} {% set existing_relation = get_or_create_relation(relation.database, relation.schema, relation.identifier, relation.type)[1] %} {% endif %} - {# {%- set relations_list = fabric__get_relation_without_caching(target_relation) -%} - {%- set existing_relation = none %} - {% if (relations_list|length == 1) and (relations_list[0][2] == target_relation.schema) - and (relations_list[0][1] == target_relation.identifier) and (relations_list[0][3] == target_relation.type)%} - {% set existing_relation = target_relation %} - {% elif (relations_list|length == 1) and (relations_list[0][2] == target_relation.schema) - and (relations_list[0][1] == target_relation.identifier) and (relations_list[0][3] != target_relation.type) %} - {% set existing_relation = get_or_create_relation(relations_list[0][0], relations_list[0][2] , relations_list[0][1] , relations_list[0][3])[1] %} - {% endif %} #} - - {# {{ log("Full refresh mode" ~ full_refresh_mode)}} - {{ log("existing relation : "~existing_relation ~ " type "~ existing_relation.type ~ " is view? "~existing_relation.is_view) }} - {{ log("target relation: " ~target_relation ~ " type "~ target_relation.type ~ " is view? "~target_relation.is_view) }} #} - - {{ log("existing relation : "~existing_relation ~ " type "~ existing_relation.type ~ " is view? "~existing_relation.is_view) }} - {{ log("target relation: " ~target_relation ~ " type "~ target_relation.type ~ " is view? "~target_relation.is_view) }} -- configs {%- set unique_key = config.get('unique_key') -%} @@ -62,8 +46,8 @@ {{ get_create_table_as_sql(True, temp_relation, sql)}} {%- endcall -%} {% do adapter.expand_target_column_types( - from_relation=temp_relation, - to_relation=target_relation) %} + from_relation=temp_relation, + to_relation=target_relation) %} {#-- Process schema changes. Returns dict of changes if successful. Use source columns for upserting/merging --#} {% set dest_columns = process_schema_changes(on_schema_change, temp_relation, existing_relation) %} {% if not dest_columns %} 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 83188c9..1fcc779 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,3 +1,10 @@ +{% macro build_columns_constraints(relation) %} + {{ return(adapter.dispatch('build_columns_constraints', 'dbt')(relation)) }} +{% endmacro %} + +{%- macro default__build_columns_constraints(relation) -%} +{%- endmacro -%} + {% 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']) -%} @@ -8,6 +15,13 @@ ) {% endmacro %} +{% macro build_model_constraints(relation) %} + {{ return(adapter.dispatch('build_columns_constraints', 'dbt')(relation)) }} +{% endmacro %} + +{%- macro default__build_model_constraints(relation) -%} +{%- 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']) -%} 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 7c281f7..9f301e3 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 @@ -1,18 +1,16 @@ {% macro fabric__create_table_as(temporary, relation, sql) -%} - {% set tmp_relation = relation.incorporate( - path={"identifier": relation.identifier.replace("#", "") ~ '_temp_view'}, - type='view')-%} - {% do adapter.drop_relation(tmp_relation) %} - {% set contract_config = config.get('contract') %} - + {% set tmp_relation = relation.incorporate( + path={"identifier": relation.identifier.replace("#", "") ~ '_temp_view'}, + type='view')-%} {{ get_create_view_as_sql(tmp_relation, sql) }} + + {% set contract_config = config.get('contract') %} {% if contract_config.enforced %} CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] {{ fabric__build_columns_constraints(relation) }} {{ get_assert_columns_equivalent(sql) }} - {% set listColumns %} {% for column in model['columns'] %} {{ "["~column~"]" }}{{ ", " if not loop.last }} @@ -23,9 +21,6 @@ ({{listColumns}}) SELECT {{listColumns}} FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]; {%- else %} - EXEC('CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] AS (SELECT * FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]);'); + EXEC('CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] AS (SELECT * FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]);'); {% endif %} - - {% do adapter.drop_relation(tmp_relation) %} - {% endmacro %} diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index ebe9530..2594baa 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -57,7 +57,7 @@ {% endif %} -- Add constraints including FK relation. - {{ fabric__build_model_constraints(target_relation) }} + {{ build_model_constraints(target_relation) }} {{ run_hooks(post_hooks, inside_transaction=False) }} {{ return({'relations': [target_relation]}) }} From 7bde13e76962802b5b7edb5a2109881e908c4223 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 15:10:26 -0700 Subject: [PATCH 10/17] moving temp relation drop to table, incremental and snapshot materializations --- .../models/incremental/incremental.sql | 11 ++++++++--- .../models/table/columns_spec_ddl.sql | 2 +- .../materializations/models/table/create_table_as.sql | 6 ++---- .../macros/materializations/snapshots/snapshot.sql | 7 +++++++ .../adapter/test_list_relations_without_caching.py | 4 ++-- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index 5fce98c..10f0248 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -22,6 +22,13 @@ {{ run_hooks(pre_hooks, inside_transaction=True) }} + -- naming a temp relation + {% set tmp_relation_view = target_relation.incorporate(path={"identifier": target_relation.identifier ~ '__dbt_tmp_vw'}, type='view')-%} + + -- Fabric & Synapse adapters use temp relation because of lack of CTE support for CTE in CTAS, Insert + -- drop temp relation if exists + {% do adapter.drop_relation(tmp_relation_view) %} + {% if existing_relation is none %} {%- call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql)}} @@ -63,14 +70,12 @@ {%- endcall -%} {% endif %} + {% do adapter.drop_relation(tmp_relation_view) %} {% do adapter.drop_relation(temp_relation) %} {{ run_hooks(post_hooks, inside_transaction=True) }} - {% set target_relation = target_relation.incorporate(type='table') %} - {% set should_revoke = should_revoke(existing_relation, full_refresh_mode) %} {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} - {% do persist_docs(target_relation, model) %} {% do adapter.commit() %} {{ return({'relations': [target_relation]}) }} 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 1fcc779..18d30bb 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 @@ -16,7 +16,7 @@ {% endmacro %} {% macro build_model_constraints(relation) %} - {{ return(adapter.dispatch('build_columns_constraints', 'dbt')(relation)) }} + {{ return(adapter.dispatch('build_model_constraints', 'dbt')(relation)) }} {% endmacro %} {%- macro default__build_model_constraints(relation) -%} 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 9f301e3..f809d13 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 @@ -1,15 +1,13 @@ {% macro fabric__create_table_as(temporary, relation, sql) -%} - {% set tmp_relation = relation.incorporate( - path={"identifier": relation.identifier.replace("#", "") ~ '_temp_view'}, - type='view')-%} + {% set tmp_relation = relation.incorporate(path={"identifier": relation.identifier ~ '__dbt_tmp_vw'}, type='view')-%} {{ get_create_view_as_sql(tmp_relation, sql) }} {% set contract_config = config.get('contract') %} {% if contract_config.enforced %} CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] - {{ fabric__build_columns_constraints(relation) }} + {{ build_columns_constraints(relation) }} {{ get_assert_columns_equivalent(sql) }} {% set listColumns %} {% for column in model['columns'] %} diff --git a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql index 286f065..bf874bb 100644 --- a/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql +++ b/dbt/include/fabric/macros/materializations/snapshots/snapshot.sql @@ -39,7 +39,14 @@ {% if not target_relation_exists %} {% set build_sql = build_snapshot_table(strategy, temp_snapshot_relation) %} + + -- naming a temp relation + {% set tmp_relation_view = target_relation.incorporate(path={"identifier": target_relation.identifier ~ '__dbt_tmp_vw'}, type='view')-%} + -- Fabric & Synapse adapters use temp relation because of lack of CTE support for CTE in CTAS, Insert + -- drop temp relation if exists + {% do adapter.drop_relation(tmp_relation_view) %} {% set final_sql = get_create_table_as_sql(False, target_relation, build_sql) %} + {% do adapter.drop_relation(tmp_relation_view) %} {% else %} diff --git a/tests/functional/adapter/test_list_relations_without_caching.py b/tests/functional/adapter/test_list_relations_without_caching.py index d728be7..5d8815a 100644 --- a/tests/functional/adapter/test_list_relations_without_caching.py +++ b/tests/functional/adapter/test_list_relations_without_caching.py @@ -119,7 +119,7 @@ def test__fabric__list_relations_without_caching(self, project): str(kwargs), ] ) - assert "n_relations: 2" in log_output + assert "n_relations: 1" in log_output class TestListRelationsWithoutCachingFull: @@ -157,4 +157,4 @@ def test__fabric__list_relations_without_caching(self, project): str(kwargs), ] ) - assert "n_relations: 12" in log_output + assert f"n_relations: {NUM_EXPECTED_RELATIONS}" in log_output From 180ecc0baa5b9bc44332b4468a76a4aeeb9aa817 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 16:58:26 -0700 Subject: [PATCH 11/17] adding drop relation to create_table_as to support test_store_tests --- .../models/table/create_table_as.sql | 1 + .../macros/materializations/tests/helpers.sql | 10 +++- .../adapter/test_store_test_failures.py | 56 ++++++------------- 3 files changed, 27 insertions(+), 40 deletions(-) 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 f809d13..00c84bd 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 @@ -21,4 +21,5 @@ {%- else %} EXEC('CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] AS (SELECT * FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]);'); {% endif %} + {% do adapter.drop_relation(tmp_relation)%} {% endmacro %} diff --git a/dbt/include/fabric/macros/materializations/tests/helpers.sql b/dbt/include/fabric/macros/materializations/tests/helpers.sql index ad8c4d8..9e4b057 100644 --- a/dbt/include/fabric/macros/materializations/tests/helpers.sql +++ b/dbt/include/fabric/macros/materializations/tests/helpers.sql @@ -1,12 +1,18 @@ {% macro fabric__get_test_sql(main_sql, fail_calc, warn_if, error_if, limit) -%} + -- Create target schema if it does not + USE [{{ target.database }}]; + IF NOT EXISTS (SELECT * FROM sys.schemas WHERE name = '{{ target.schema }}') + BEGIN + EXEC('CREATE SCHEMA [{{ target.schema }}]') + END + {% if main_sql.strip().lower().startswith('with') %} {% set testview %} - {{ config.get('schema') }}.testview_{{ range(1300, 19000) | random }} + {{ target.schema }}.testview_{{ range(1300, 19000) | random }} {% endset %} {% set sql = main_sql.replace("'", "''")%} - EXEC('create view {{testview}} as {{ sql }};') select {{ "top (" ~ limit ~ ')' if limit != none }} diff --git a/tests/functional/adapter/test_store_test_failures.py b/tests/functional/adapter/test_store_test_failures.py index e079e3f..6a2fdbf 100644 --- a/tests/functional/adapter/test_store_test_failures.py +++ b/tests/functional/adapter/test_store_test_failures.py @@ -1,33 +1,17 @@ import pytest -from dbt.tests.adapter.store_test_failures_tests import basic -from dbt.tests.adapter.store_test_failures_tests.fixtures import ( - models__file_model_but_with_a_no_good_very_long_name, - models__fine_model, - models__problematic_model, - properties__schema_yml, - seeds__expected_accepted_values, - seeds__expected_failing_test, - seeds__expected_not_null_problematic_model_id, - seeds__expected_unique_problematic_model_id, - seeds__people, - tests__failing_test, -) +from dbt.tests.adapter.store_test_failures_tests import basic, fixtures from dbt.tests.util import check_relations_equal, run_dbt -# from dbt.tests.adapter.store_test_failures_tests.test_store_test_failures import ( -# TestStoreTestFailures, -# ) +# used to rename test audit schema to help test schema meet max char limit +# the default is _dbt_test__audit but this runs over the postgres 63 schema name char limit +# without which idempotency conditions will not hold (i.e. dbt can't drop the schema properly) +TEST_AUDIT_SCHEMA_SUFFIX = "dbt_test__aud" tests__passing_test = """ select * from {{ ref('fine_model') }} where 1=2 """ -# used to rename test audit schema to help test schema meet max char limit -# the default is _dbt_test__audit but this runs over the postgres 63 schema name char limit -# without which idempotency conditions will not hold (i.e. dbt can't drop the schema properly) -TEST_AUDIT_SCHEMA_SUFFIX = "dbt_test__aud" - class StoreTestFailuresBase: @pytest.fixture(scope="function", autouse=True) @@ -39,30 +23,30 @@ def setUp(self, project): @pytest.fixture(scope="class") def seeds(self): return { - "people.csv": seeds__people, - "expected_accepted_values.csv": seeds__expected_accepted_values, - "expected_failing_test.csv": seeds__expected_failing_test, - "expected_not_null_problematic_model_id.csv": seeds__expected_not_null_problematic_model_id, - "expected_unique_problematic_model_id.csv": seeds__expected_unique_problematic_model_id, + "people.csv": fixtures.seeds__people, + "expected_accepted_values.csv": fixtures.seeds__expected_accepted_values, + "expected_failing_test.csv": fixtures.seeds__expected_failing_test, + "expected_not_null_problematic_model_id.csv": fixtures.seeds__expected_not_null_problematic_model_id, + "expected_unique_problematic_model_id.csv": fixtures.seeds__expected_unique_problematic_model_id, } @pytest.fixture(scope="class") def tests(self): return { - "failing_test.sql": tests__failing_test, + "failing_test.sql": fixtures.tests__failing_test, "passing_test.sql": tests__passing_test, } @pytest.fixture(scope="class") def properties(self): - return {"schema.yml": properties__schema_yml} + return {"schema.yml": fixtures.properties__schema_yml} @pytest.fixture(scope="class") def models(self): return { - "fine_model.sql": models__fine_model, - "fine_model_but_with_a_no_good_very_long_name.sql": models__file_model_but_with_a_no_good_very_long_name, - "problematic_model.sql": models__problematic_model, + "fine_model.sql": fixtures.models__fine_model, + "fine_model_but_with_a_no_good_very_long_name.sql": fixtures.models__file_model_but_with_a_no_good_very_long_name, + "problematic_model.sql": fixtures.models__problematic_model, } @pytest.fixture(scope="class") @@ -72,7 +56,7 @@ def project_config_update(self): "quote_columns": False, "test": self.column_type_overrides(), }, - "tests": {"+schema": TEST_AUDIT_SCHEMA_SUFFIX}, + "data_tests": {"+schema": TEST_AUDIT_SCHEMA_SUFFIX}, } def column_type_overrides(self): @@ -137,7 +121,7 @@ def run_tests_store_failures_and_assert(self, project): ) -class TestStoreTestFailures(StoreTestFailuresBase): +class BaseStoreTestFailures(StoreTestFailuresBase): @pytest.fixture(scope="function") def clean_up(self, project): yield @@ -171,11 +155,7 @@ def test__store_and_assert(self, project, clean_up): self.run_tests_store_failures_and_assert(project) -class TestFabricStoreTestFailures(TestStoreTestFailures): - pass - - -class TestStoreTestFailuresAsInteractions(basic.StoreTestFailuresAsInteractions): +class TestStoreTestFailures(BaseStoreTestFailures): pass From 362e415c01c848712b0aa2702c03034f87b7b8c1 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 19:24:15 -0700 Subject: [PATCH 12/17] adding log statements --- .../fabric/macros/materializations/models/table/table.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index 2594baa..075aea7 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -2,7 +2,7 @@ -- Load target relation {%- set target_relation = this.incorporate(type='table') %} - {%- set existing_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} + {%- set existing_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} {%- set backup_relation = none %} {% if (existing_relation != none and existing_relation.type == "table") %} @@ -32,7 +32,7 @@ -- Fabric & Synapse adapters use temp relation because of lack of CTE support for CTE in CTAS, Insert -- drop temp relation if exists {% do adapter.drop_relation(tmp_relation) %} - + {{ log("printing target relation - "~target_relation)}} -- build model {% call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql) }} From a13807a1a9cc8381c04c71adb9860f7d79c9d67c Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 20:47:24 -0700 Subject: [PATCH 13/17] removing comments --- .../fabric/macros/materializations/models/table/table.sql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql index 075aea7..88f1df9 100644 --- a/dbt/include/fabric/macros/materializations/models/table/table.sql +++ b/dbt/include/fabric/macros/materializations/models/table/table.sql @@ -12,7 +12,6 @@ {% endif %} {% if (existing_relation != none) %} - -- drop the temp relations if they exist already in the database {% do adapter.drop_relation(backup_relation) %} -- Rename target relation as backup relation @@ -32,7 +31,7 @@ -- Fabric & Synapse adapters use temp relation because of lack of CTE support for CTE in CTAS, Insert -- drop temp relation if exists {% do adapter.drop_relation(tmp_relation) %} - {{ log("printing target relation - "~target_relation)}} + -- build model {% call statement('main') -%} {{ get_create_table_as_sql(False, target_relation, sql) }} From abb9131c05cd7231396e0d6317e5c582febac6e1 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Mon, 27 May 2024 21:35:36 -0700 Subject: [PATCH 14/17] Update --- .github/workflows/integration-tests-azure.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integration-tests-azure.yml b/.github/workflows/integration-tests-azure.yml index 8d40135..0691777 100644 --- a/.github/workflows/integration-tests-azure.yml +++ b/.github/workflows/integration-tests-azure.yml @@ -11,11 +11,12 @@ jobs: name: Regular strategy: fail-fast: false + max-parallel: 1 matrix: - python_version: ["3.8", "3.9", "3.10", "3.11"] profile: ["ci_azure_auto"] + python_version: ["3.11"] msodbc_version: ["17", "18"] - max-parallel: 1 + runs-on: ubuntu-latest container: image: ghcr.io/${{ github.repository }}:CI-${{ matrix.python_version }}-msodbc${{ matrix.msodbc_version }} From 59e81b8f55ae4ece9da33918bcbe60ce01652909 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Thu, 30 May 2024 09:56:09 -0700 Subject: [PATCH 15/17] Update dbt/include/fabric/macros/adapters/columns.sql Co-authored-by: Jeremy Cohen --- dbt/include/fabric/macros/adapters/columns.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/fabric/macros/adapters/columns.sql b/dbt/include/fabric/macros/adapters/columns.sql index 522cc9c..69f2ce8 100644 --- a/dbt/include/fabric/macros/adapters/columns.sql +++ b/dbt/include/fabric/macros/adapters/columns.sql @@ -54,8 +54,8 @@ {% macro fabric__alter_column_type(relation, column_name, new_column_type) %} - {%- set table_name= relation.include(database=False).include(schema=False)-%} - {%- set schema_name = relation.include(database=False).include(identifier=False) -%} + {%- set table_name= relation.identifier -%} + {%- set schema_name = relation.schema -%} {% set generate_tmp_relation_script %} SELECT TRIM(REPLACE(STRING_AGG(ColumnName + ' ', ',-'), '-', CHAR(10))) AS ColumnDef From f172f8c091fb3cb250ee3baa845f38013602e0ca Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Thu, 30 May 2024 10:14:07 -0700 Subject: [PATCH 16/17] Update dbt/include/fabric/macros/materializations/models/incremental/incremental.sql Co-authored-by: Jeremy Cohen --- .../macros/materializations/models/incremental/incremental.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index 10f0248..a172f04 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -3,7 +3,7 @@ {%- set full_refresh_mode = (should_full_refresh()) -%} {% set target_relation = this.incorporate(type='table') %} - {%- set relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%} +{%- set relation = load_cached_relation(this) -%} {%- set existing_relation = none %} {% if (relation.type == target_relation.type) and (relation.identifier == target_relation.identifier) and (relation.schema == target_relation.schema) and (relation.database == target_relation.database) %} From 0f8e3cbb58daada4cf135a690a94f37c4312c852 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Thu, 30 May 2024 12:56:32 -0700 Subject: [PATCH 17/17] Resolving comments --- dbt/adapters/fabric/fabric_connection_manager.py | 6 ++++++ dbt/adapters/fabric/fabric_credentials.py | 3 +++ dbt/include/fabric/macros/adapters/metadata.sql | 4 ---- dbt/include/fabric/macros/adapters/relation.sql | 7 ++++--- .../materializations/models/incremental/incremental.sql | 7 +++---- .../materializations/models/table/columns_spec_ddl.sql | 6 ------ tests/conftest.py | 1 + tests/functional/adapter/test_constraints.py | 2 -- 8 files changed, 17 insertions(+), 19 deletions(-) diff --git a/dbt/adapters/fabric/fabric_connection_manager.py b/dbt/adapters/fabric/fabric_connection_manager.py index 9719d84..f4130fa 100644 --- a/dbt/adapters/fabric/fabric_connection_manager.py +++ b/dbt/adapters/fabric/fabric_connection_manager.py @@ -323,6 +323,12 @@ def open(cls, connection: Connection) -> Connection: con_str.append(f"Database={credentials.database}") + #Enabling trace flag + if credentials.trace_flag: + con_str.append("SQL_ATTR_TRACE=SQL_OPT_TRACE_ON") + else: + con_str.append("SQL_ATTR_TRACE=SQL_OPT_TRACE_OFF") + assert credentials.authentication is not None if "ActiveDirectory" in credentials.authentication: diff --git a/dbt/adapters/fabric/fabric_credentials.py b/dbt/adapters/fabric/fabric_credentials.py index dec36fa..a824fac 100644 --- a/dbt/adapters/fabric/fabric_credentials.py +++ b/dbt/adapters/fabric/fabric_credentials.py @@ -13,6 +13,7 @@ class FabricCredentials(Credentials): UID: Optional[str] = None PWD: Optional[str] = None windows_login: Optional[bool] = False + trace_flag: Optional[bool] = False tenant_id: Optional[str] = None client_id: Optional[str] = None client_secret: Optional[str] = None @@ -36,6 +37,7 @@ class FabricCredentials(Credentials): "app_secret": "client_secret", "TrustServerCertificate": "trust_cert", "schema_auth": "schema_authorization", + "SQL_ATTR_TRACE": "trace_flag", } @property @@ -63,6 +65,7 @@ def _connection_keys(self): "retries", "login_timeout", "query_timeout", + "trace_flag", ) @property diff --git a/dbt/include/fabric/macros/adapters/metadata.sql b/dbt/include/fabric/macros/adapters/metadata.sql index 4f4fa0a..2fe6a58 100644 --- a/dbt/include/fabric/macros/adapters/metadata.sql +++ b/dbt/include/fabric/macros/adapters/metadata.sql @@ -13,10 +13,6 @@ {{ return(adapter.dispatch('get_use_database_sql', 'dbt')(database)) }} {% endmacro %} -{%- macro default__get_use_database_sql(database) -%} -{%- endmacro -%} - - {%- macro fabric__get_use_database_sql(database) -%} USE [{{database}}]; {%- endmacro -%} diff --git a/dbt/include/fabric/macros/adapters/relation.sql b/dbt/include/fabric/macros/adapters/relation.sql index 3fa4ff8..a6fe0f0 100644 --- a/dbt/include/fabric/macros/adapters/relation.sql +++ b/dbt/include/fabric/macros/adapters/relation.sql @@ -27,9 +27,10 @@ {% set references = load_result('find_references')['data'] %} {% for reference in references -%} -- dropping referenced view {{ reference[0] }}.{{ reference[1] }} - {% do adapter.drop_relation(relation.incorporate( - type="view", - path={"schema": reference[0], "identifier": reference[1]})) %} + {% do adapter.drop_relation + (api.Relation.create( + identifier = reference[1], schema = reference[0], database = relation.database, type='view' + ))%} {% endfor %} {% elif relation.type == 'table'%} {% set object_id_type = 'U' %} diff --git a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql index a172f04..bd045df 100644 --- a/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql +++ b/dbt/include/fabric/macros/materializations/models/incremental/incremental.sql @@ -1,14 +1,13 @@ - {% materialization incremental, adapter='fabric' -%} {%- set full_refresh_mode = (should_full_refresh()) -%} {% set target_relation = this.incorporate(type='table') %} -{%- set relation = load_cached_relation(this) -%} + {%- set relation = load_cached_relation(this) -%} {%- set existing_relation = none %} - {% if (relation.type == target_relation.type) and (relation.identifier == target_relation.identifier) and (relation.schema == target_relation.schema) and (relation.database == target_relation.database) %} + {% if relation.type == 'table' %} {% set existing_relation = target_relation %} - {% elif (relation.type != target_relation.type) and (relation.identifier == target_relation.identifier) and (relation.schema == target_relation.schema) and (relation.database == target_relation.database) %} + {% elif relation.type == 'view' %} {% set existing_relation = get_or_create_relation(relation.database, relation.schema, relation.identifier, relation.type)[1] %} {% 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 18d30bb..e8307d8 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 @@ -2,9 +2,6 @@ {{ return(adapter.dispatch('build_columns_constraints', 'dbt')(relation)) }} {% endmacro %} -{%- macro default__build_columns_constraints(relation) -%} -{%- endmacro -%} - {% 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']) -%} @@ -19,9 +16,6 @@ {{ return(adapter.dispatch('build_model_constraints', 'dbt')(relation)) }} {% endmacro %} -{%- macro default__build_model_constraints(relation) -%} -{%- 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']) -%} diff --git a/tests/conftest.py b/tests/conftest.py index 1e3670c..3e60ce0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -55,6 +55,7 @@ def _profile_ci_azure_base(): "database": os.getenv("DBT_AZURESQL_DB"), "encrypt": True, "trust_cert": True, + "trace_flag":False, }, } diff --git a/tests/functional/adapter/test_constraints.py b/tests/functional/adapter/test_constraints.py index 8ea6ce6..6759424 100644 --- a/tests/functional/adapter/test_constraints.py +++ b/tests/functional/adapter/test_constraints.py @@ -568,8 +568,6 @@ def test__model_constraints_ddl(self, project, expected_sql): generated_sql_generic, "foreign_key_model", "" ) generated_sql_wodb = generated_sql_generic.replace("USE [" + project.database + "];", "") - print("Expected:", expected_sql.strip()) - print("Generated:", generated_sql_wodb.strip()) assert _normalize_whitespace(expected_sql.strip()) == _normalize_whitespace( generated_sql_wodb.strip() )