From 091c3bbd5d360df7d356f05e8e6153ece56b0c52 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Mon, 9 Dec 2024 12:06:55 -0800 Subject: [PATCH 1/4] Add failing test. --- .../dynamic_table_tests/models.py | 11 +++++++ .../dynamic_table_tests/test_basic.py | 32 ++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/functional/relation_tests/dynamic_table_tests/models.py b/tests/functional/relation_tests/dynamic_table_tests/models.py index 4dcd6cf48..b1ed0dbf2 100644 --- a/tests/functional/relation_tests/dynamic_table_tests/models.py +++ b/tests/functional/relation_tests/dynamic_table_tests/models.py @@ -17,6 +17,17 @@ """ +AUTO_DYNAMIC_TABLE = """ +{{ config( + materialized='dynamic_table', + snowflake_warehouse='DBT_TESTING', + target_lag='2 minutes', + refresh_mode='AUTO', +) }} +select * from {{ ref('my_seed') }} +""" + + DYNAMIC_TABLE_DOWNSTREAM = """ {{ config( materialized='dynamic_table', diff --git a/tests/functional/relation_tests/dynamic_table_tests/test_basic.py b/tests/functional/relation_tests/dynamic_table_tests/test_basic.py index 79a2241ca..450ba0970 100644 --- a/tests/functional/relation_tests/dynamic_table_tests/test_basic.py +++ b/tests/functional/relation_tests/dynamic_table_tests/test_basic.py @@ -1,6 +1,6 @@ import pytest -from dbt.tests.util import run_dbt +from dbt.tests.util import assert_message_in_logs, run_dbt, run_dbt_and_capture from tests.functional.relation_tests.dynamic_table_tests import models from tests.functional.utils import query_relation_type @@ -46,3 +46,33 @@ class TestBasicIcebergOn(TestBasic): @pytest.fixture(scope="class") def project_config_update(self): return {"flags": {"enable_iceberg_materializations": True}} + + +class TestAutoConfigDoesntRebuild: + DT_NAME = "my_dynamic_table" + + @pytest.fixture(scope="class", autouse=True) + def seeds(self): + return {"my_seed.csv": models.SEED} + + @pytest.fixture(scope="class", autouse=True) + def models(self): + yield { + f"{self.DT_NAME}.sql": models.AUTO_DYNAMIC_TABLE, + } + + def test_auto_config_doesnt_rebuild(self, project): + model_qualified_name = f"{project.database}.{project.test_schema}.{self.DT_NAME}" + + run_dbt(["seed"]) + _, logs = run_dbt_and_capture(["--debug", "run", "--select", f"{self.DT_NAME}.sql"]) + assert_message_in_logs(f"create dynamic table {model_qualified_name}", logs) + assert_message_in_logs("refresh_mode = AUTO", logs) + + _, logs = run_dbt_and_capture(["--debug", "run", "--select", f"{self.DT_NAME}.sql"]) + assert_message_in_logs(f"create dynamic table {model_qualified_name}", logs, False) + assert_message_in_logs("refresh_mode = AUTO", logs, False) + assert_message_in_logs( + f"No configuration changes were identified on on: `{model_qualified_name}`. Continuing.", + logs, + ) From 4d78bf5be0f12270359e2d1eb1468abac74df4c7 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Mon, 9 Dec 2024 12:19:02 -0800 Subject: [PATCH 2/4] Parametrize cases --- .../relation_tests/dynamic_table_tests/models.py | 11 ++++++++++- .../relation_tests/dynamic_table_tests/test_basic.py | 12 +++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/functional/relation_tests/dynamic_table_tests/models.py b/tests/functional/relation_tests/dynamic_table_tests/models.py index b1ed0dbf2..57d83f968 100644 --- a/tests/functional/relation_tests/dynamic_table_tests/models.py +++ b/tests/functional/relation_tests/dynamic_table_tests/models.py @@ -17,7 +17,7 @@ """ -AUTO_DYNAMIC_TABLE = """ +EXPLICIT_AUTO_DYNAMIC_TABLE = """ {{ config( materialized='dynamic_table', snowflake_warehouse='DBT_TESTING', @@ -27,6 +27,15 @@ select * from {{ ref('my_seed') }} """ +IMPLICIT_AUTO_DYNAMIC_TABLE = """ +{{ config( + materialized='dynamic_table', + snowflake_warehouse='DBT_TESTING', + target_lag='2 minutes', +) }} +select * from {{ ref('my_seed') }} +""" + DYNAMIC_TABLE_DOWNSTREAM = """ {{ config( diff --git a/tests/functional/relation_tests/dynamic_table_tests/test_basic.py b/tests/functional/relation_tests/dynamic_table_tests/test_basic.py index 450ba0970..4d2cf2fec 100644 --- a/tests/functional/relation_tests/dynamic_table_tests/test_basic.py +++ b/tests/functional/relation_tests/dynamic_table_tests/test_basic.py @@ -58,18 +58,20 @@ def seeds(self): @pytest.fixture(scope="class", autouse=True) def models(self): yield { - f"{self.DT_NAME}.sql": models.AUTO_DYNAMIC_TABLE, + f"explicit_{self.DT_NAME}.sql": models.EXPLICIT_AUTO_DYNAMIC_TABLE, + f"implicit_{self.DT_NAME}.sql": models.IMPLICIT_AUTO_DYNAMIC_TABLE, } - def test_auto_config_doesnt_rebuild(self, project): - model_qualified_name = f"{project.database}.{project.test_schema}.{self.DT_NAME}" + @pytest.mark.parametrize("test_dt", [f"explicit_{DT_NAME}", f"implicit_{DT_NAME}"]) + def test_auto_config_doesnt_rebuild(self, project, test_dt): + model_qualified_name = f"{project.database}.{project.test_schema}.{test_dt}" run_dbt(["seed"]) - _, logs = run_dbt_and_capture(["--debug", "run", "--select", f"{self.DT_NAME}.sql"]) + _, logs = run_dbt_and_capture(["--debug", "run", "--select", f"{test_dt}.sql"]) assert_message_in_logs(f"create dynamic table {model_qualified_name}", logs) assert_message_in_logs("refresh_mode = AUTO", logs) - _, logs = run_dbt_and_capture(["--debug", "run", "--select", f"{self.DT_NAME}.sql"]) + _, logs = run_dbt_and_capture(["--debug", "run", "--select", f"{test_dt}.sql"]) assert_message_in_logs(f"create dynamic table {model_qualified_name}", logs, False) assert_message_in_logs("refresh_mode = AUTO", logs, False) assert_message_in_logs( From c7f1203d3e26232d4718e293467be29dbb864680 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Mon, 9 Dec 2024 13:10:38 -0800 Subject: [PATCH 3/4] Change comparison logic for when to rebuild if DT has auto refresh strategy (implicit or explicit). --- dbt/adapters/snowflake/relation.py | 6 +++++- .../snowflake/relation_configs/__init__.py | 1 + .../dynamic_table_tests/test_basic.py | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index b6924b9b3..54db21924 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -17,6 +17,7 @@ from dbt_common.events.functions import fire_event, warn_or_error from dbt.adapters.snowflake.relation_configs import ( + RefreshMode, SnowflakeCatalogConfigChange, SnowflakeDynamicTableConfig, SnowflakeDynamicTableConfigChangeset, @@ -109,7 +110,10 @@ def dynamic_table_config_changeset( ) ) - if new_dynamic_table.refresh_mode != existing_dynamic_table.refresh_mode: + if ( + new_dynamic_table.refresh_mode != RefreshMode.AUTO + and new_dynamic_table.refresh_mode != existing_dynamic_table.refresh_mode + ): config_change_collection.refresh_mode = SnowflakeDynamicTableRefreshModeConfigChange( action=RelationConfigChangeAction.create, context=new_dynamic_table.refresh_mode, diff --git a/dbt/adapters/snowflake/relation_configs/__init__.py b/dbt/adapters/snowflake/relation_configs/__init__.py index fec9d8a54..67f7644d2 100644 --- a/dbt/adapters/snowflake/relation_configs/__init__.py +++ b/dbt/adapters/snowflake/relation_configs/__init__.py @@ -3,6 +3,7 @@ SnowflakeCatalogConfigChange, ) from dbt.adapters.snowflake.relation_configs.dynamic_table import ( + RefreshMode, SnowflakeDynamicTableConfig, SnowflakeDynamicTableConfigChangeset, SnowflakeDynamicTableRefreshModeConfigChange, diff --git a/tests/functional/relation_tests/dynamic_table_tests/test_basic.py b/tests/functional/relation_tests/dynamic_table_tests/test_basic.py index 4d2cf2fec..8cdf59ebc 100644 --- a/tests/functional/relation_tests/dynamic_table_tests/test_basic.py +++ b/tests/functional/relation_tests/dynamic_table_tests/test_basic.py @@ -48,7 +48,12 @@ def project_config_update(self): return {"flags": {"enable_iceberg_materializations": True}} -class TestAutoConfigDoesntRebuild: +class TestAutoConfigDoesntFullRefresh: + """ + AUTO refresh_strategy will be compared accurately with both INCREMENTAL and FULL. + https://github.com/dbt-labs/dbt-snowflake/issues/1267 + """ + DT_NAME = "my_dynamic_table" @pytest.fixture(scope="class", autouse=True) @@ -63,7 +68,7 @@ def models(self): } @pytest.mark.parametrize("test_dt", [f"explicit_{DT_NAME}", f"implicit_{DT_NAME}"]) - def test_auto_config_doesnt_rebuild(self, project, test_dt): + def test_auto_config_doesnt_full_refresh(self, project, test_dt): model_qualified_name = f"{project.database}.{project.test_schema}.{test_dt}" run_dbt(["seed"]) @@ -72,9 +77,13 @@ def test_auto_config_doesnt_rebuild(self, project, test_dt): assert_message_in_logs("refresh_mode = AUTO", logs) _, logs = run_dbt_and_capture(["--debug", "run", "--select", f"{test_dt}.sql"]) + assert_message_in_logs(f"create dynamic table {model_qualified_name}", logs, False) + assert_message_in_logs( + f"create or replace dynamic table {model_qualified_name}", logs, False + ) assert_message_in_logs("refresh_mode = AUTO", logs, False) assert_message_in_logs( - f"No configuration changes were identified on on: `{model_qualified_name}`. Continuing.", + f"No configuration changes were identified on: `{model_qualified_name}`. Continuing.", logs, ) From b9c9f439b7a20b52728854e6a7852a62df24e778 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Mon, 9 Dec 2024 13:16:01 -0800 Subject: [PATCH 4/4] Add changelog --- .changes/unreleased/Fixes-20241209-131530.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20241209-131530.yaml diff --git a/.changes/unreleased/Fixes-20241209-131530.yaml b/.changes/unreleased/Fixes-20241209-131530.yaml new file mode 100644 index 000000000..b3196a3b0 --- /dev/null +++ b/.changes/unreleased/Fixes-20241209-131530.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: AUTO should no longer lead to rebuilds of dynamic tables. +time: 2024-12-09T13:15:30.554566-08:00 +custom: + Author: versusfacit + Issue: "1267"