From 35110fe6fed26a6a5f74e2a11ad6a7dcd5377133 Mon Sep 17 00:00:00 2001 From: Pradeep Srikakolapu Date: Thu, 23 May 2024 02:38:58 -0700 Subject: [PATCH] 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 db081ff8..522cc9c9 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 4899c954..3fa4ff83 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 f85c0567..a117e47c 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 49547ff1..698b2134 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 b2e9d245..b552e454 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 dcaadc5c..7c281f70 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 2977dc33..84aefd60 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 40f77525..e4aa8dca 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 1d496e80..286f0654 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 f5d43b05..b426258f 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 65a1b490..ad833ca4 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 b0b6bcd1..82984fa9 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 10906a1c..8af58cb7 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 da1676aa..a5727455 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 f581fa92..d728be73 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 7e4781a5..98fc95ba 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 1439ee5c..e079e3f7 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