From 2d34916a9f0b38e9cc2b15315b4b316dc7c05534 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Tue, 21 May 2024 23:40:10 -0700 Subject: [PATCH] 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