From 9b8e04c0c0ee3f8a73123f64daa20344ee6c8c71 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 8 Oct 2024 12:23:04 -0700 Subject: [PATCH 1/6] Fix base location not rendering without subpath and add tests. Take optional off params that are not optional in dynamic table create DDL. --- .../snowflake/relation_configs/catalog.py | 8 +- .../macros/relations/dynamic_table/create.sql | 6 +- .../relations/dynamic_table/replace.sql | 6 +- tests/functional/iceberg/models.py | 85 +++++++++++++++++++ tests/functional/iceberg/test_table_basic.py | 72 +++------------- 5 files changed, 109 insertions(+), 68 deletions(-) create mode 100644 tests/functional/iceberg/models.py diff --git a/dbt/adapters/snowflake/relation_configs/catalog.py b/dbt/adapters/snowflake/relation_configs/catalog.py index 09e338635..c8d7de40f 100644 --- a/dbt/adapters/snowflake/relation_configs/catalog.py +++ b/dbt/adapters/snowflake/relation_configs/catalog.py @@ -1,5 +1,5 @@ from dataclasses import dataclass -from typing import Any, Dict, Optional, TYPE_CHECKING, Set +from typing import Any, Dict, Optional, TYPE_CHECKING, Set, List if TYPE_CHECKING: import agate @@ -82,8 +82,10 @@ def parse_relation_config(cls, relation_config: RelationConfig) -> Dict[str, Any if external_volume := relation_config.config.extra.get("external_volume"): config_dict["external_volume"] = external_volume - if base_location := relation_config.config.extra.get("base_location_subpath"): - config_dict["base_location"] = base_location + catalog_dirs: List[str] = ["_dbt", relation_config.schema, relation_config.name] + if base_location_subpath := relation_config.config.extra.get("base_location_subpath"): + catalog_dirs.append(base_location_subpath) + config_dict["base_location"] = "/".join(catalog_dirs) return config_dict diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql index 0bd190dcc..92fc53a26 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql @@ -73,9 +73,9 @@ create dynamic iceberg table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - {{ optional('external_volume', dynamic_table.catalog.external_volume) }} - {{ optional('catalog', dynamic_table.catalog.name) }} - base_location = {{ dynamic_table.catalog.base_location }} + external_volume = '{{ dynamic_table.catalog.external_volume }}' + catalog = '{{ dynamic_table.catalog.name }}' + base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} {{ optional('initialize', dynamic_table.initialize) }} as ( diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql index f9ba1275a..f7ea0619d 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql @@ -72,9 +72,9 @@ create or replace dynamic iceberg table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - {{ optional('external_volume', dynamic_table.catalog.external_volume) }} - {{ optional('catalog', dynamic_table.catalog.name) }} - base_location = {{ dynamic_table.catalog.base_location }} + external_volume = '{{ dynamic_table.catalog.external_volume }}' + catalog = '{{ dynamic_table.catalog.name }}' + base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} {{ optional('initialize', dynamic_table.initialize) }} as ( diff --git a/tests/functional/iceberg/models.py b/tests/functional/iceberg/models.py new file mode 100644 index 000000000..6433f74bf --- /dev/null +++ b/tests/functional/iceberg/models.py @@ -0,0 +1,85 @@ +_MODEL_BASIC_TABLE_MODEL = """ +{{ + config( + materialized = "table", + cluster_by=['id'], + ) +}} +select 1 as id +""" + +_MODEL_BASIC_ICEBERG_MODEL = """ +{{ + config( + transient = "true", + materialized = "table", + cluster_by=['id'], + table_format="iceberg", + external_volume="s3_iceberg_snow", + base_location_subpath="subpath", + ) +}} + +select * from {{ ref('first_table') }} +""" + +_MODEL_BASIC_DYNAMIC_TABLE_MODEL = """ +{{ config( + materialized='dynamic_table', + snowflake_warehouse='DBT_TESTING', + target_lag='1 minute', + refresh_mode='INCREMENTAL', + table_format='iceberg', + external_volume='s3_iceberg_snow', +) }} + +select * from {{ ref('first_table') }} +""" + +_MODEL_BASIC_DYNAMIC_TABLE_MODEL_WITH_SUBPATH = """ +{{ config( + materialized='dynamic_table', + snowflake_warehouse='DBT_TESTING', + target_lag='1 minute', + refresh_mode='INCREMENTAL', + table_format='iceberg', + external_volume='s3_iceberg_snow', + base_location_subpath='subpath', +) }} + +select * from {{ ref('first_table') }} +""" + +_MODEL_BUILT_ON_ICEBERG_TABLE = """ +{{ + config( + materialized = "table", + ) +}} +select * from {{ ref('iceberg_table') }} +""" + +_MODEL_TABLE_BEFORE_SWAP = """ +{{ + config( + materialized = "table", + ) +}} +select 1 as id +""" + +_MODEL_VIEW_BEFORE_SWAP = """ +select 1 as id +""" + +_MODEL_TABLE_FOR_SWAP_ICEBERG = """ +{{ + config( + materialized = "table", + table_format="iceberg", + external_volume="s3_iceberg_snow", + base_location_subpath="subpath", + ) +}} +select 1 as id +""" diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py index 0bfdf59f1..e835a5fce 100644 --- a/tests/functional/iceberg/test_table_basic.py +++ b/tests/functional/iceberg/test_table_basic.py @@ -4,64 +4,16 @@ from dbt.tests.util import run_dbt, rm_file, write_file -_MODEL_BASIC_TABLE_MODEL = """ -{{ - config( - materialized = "table", - cluster_by=['id'], - ) -}} -select 1 as id -""" - -_MODEL_BASIC_ICEBERG_MODEL = """ -{{ - config( - transient = "true", - materialized = "table", - cluster_by=['id'], - table_format="iceberg", - external_volume="s3_iceberg_snow", - base_location_subpath="subpath", - ) -}} - -select * from {{ ref('first_table') }} -""" - -_MODEL_BUILT_ON_ICEBERG_TABLE = """ -{{ - config( - materialized = "table", - ) -}} -select * from {{ ref('iceberg_table') }} -""" - -_MODEL_TABLE_BEFORE_SWAP = """ -{{ - config( - materialized = "table", - ) -}} -select 1 as id -""" - -_MODEL_VIEW_BEFORE_SWAP = """ -select 1 as id -""" - -_MODEL_TABLE_FOR_SWAP_ICEBERG = """ -{{ - config( - materialized = "table", - table_format="iceberg", - external_volume="s3_iceberg_snow", - base_location_subpath="subpath", - ) -}} -select 1 as id -""" +from tests.functional.iceberg.models import ( + _MODEL_BASIC_TABLE_MODEL, + _MODEL_BASIC_ICEBERG_MODEL, + _MODEL_BASIC_DYNAMIC_TABLE_MODEL, + _MODEL_BASIC_DYNAMIC_TABLE_MODEL_WITH_SUBPATH, + _MODEL_BUILT_ON_ICEBERG_TABLE, + _MODEL_TABLE_BEFORE_SWAP, + _MODEL_VIEW_BEFORE_SWAP, + _MODEL_TABLE_FOR_SWAP_ICEBERG, +) class TestIcebergTableBuilds: @@ -75,11 +27,13 @@ def models(self): "first_table.sql": _MODEL_BASIC_TABLE_MODEL, "iceberg_table.sql": _MODEL_BASIC_ICEBERG_MODEL, "table_built_on_iceberg_table.sql": _MODEL_BUILT_ON_ICEBERG_TABLE, + "dynamic_table.sql": _MODEL_BASIC_DYNAMIC_TABLE_MODEL, + "dynamic_tableb.sql": _MODEL_BASIC_DYNAMIC_TABLE_MODEL_WITH_SUBPATH, } def test_iceberg_tables_build_and_can_be_referred(self, project): run_results = run_dbt() - assert len(run_results) == 3 + assert len(run_results) == 5 class TestIcebergTableTypeBuildsOnExistingTable: From 5b20f1583cab6fa1648a6b1bf15d7f63e66f2b0b Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 8 Oct 2024 12:26:42 -0700 Subject: [PATCH 2/6] Add changelog. --- .changes/unreleased/Fixes-20241008-122635.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Fixes-20241008-122635.yaml diff --git a/.changes/unreleased/Fixes-20241008-122635.yaml b/.changes/unreleased/Fixes-20241008-122635.yaml new file mode 100644 index 000000000..90cc72995 --- /dev/null +++ b/.changes/unreleased/Fixes-20241008-122635.yaml @@ -0,0 +1,6 @@ +kind: Fixes +body: Dynamic Iceberg table DDL catalog parameter fixes. +time: 2024-10-08T12:26:35.521308-07:00 +custom: + Author: versusfacit + Issue: "1200" From 40c33e32498d439d17c6087e5dcff17bb37017a0 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 8 Oct 2024 14:38:52 -0700 Subject: [PATCH 3/6] Revert changes to external volume --- dbt/include/snowflake/macros/relations/dynamic_table/create.sql | 2 +- .../snowflake/macros/relations/dynamic_table/replace.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql index 92fc53a26..8b7710a77 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql @@ -73,7 +73,7 @@ create dynamic iceberg table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - external_volume = '{{ dynamic_table.catalog.external_volume }}' + {{ optional('external_volume', dynamic_table.catalog.external_volume) }} catalog = '{{ dynamic_table.catalog.name }}' base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql index f7ea0619d..0a4e0b165 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql @@ -72,7 +72,7 @@ create or replace dynamic iceberg table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - external_volume = '{{ dynamic_table.catalog.external_volume }}' + {{ optional('external_volume', dynamic_table.catalog.external_volume) }} catalog = '{{ dynamic_table.catalog.name }}' base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} From ca430cccee06dfad27a730c6c30b344c6a4cb199 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 8 Oct 2024 15:09:26 -0700 Subject: [PATCH 4/6] revert changes to catalog optionality. --- dbt/include/snowflake/macros/relations/dynamic_table/create.sql | 2 +- .../snowflake/macros/relations/dynamic_table/replace.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql index 8b7710a77..4078a7dd6 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql @@ -74,7 +74,7 @@ target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} {{ optional('external_volume', dynamic_table.catalog.external_volume) }} - catalog = '{{ dynamic_table.catalog.name }}' + {{ optional('catalog', dynamic_table.catalog.name) }} base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} {{ optional('initialize', dynamic_table.initialize) }} diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql index 0a4e0b165..bcfcffad5 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql @@ -73,7 +73,7 @@ target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} {{ optional('external_volume', dynamic_table.catalog.external_volume) }} - catalog = '{{ dynamic_table.catalog.name }}' + {{ optional('catalog', dynamic_table.catalog.name) }} base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} {{ optional('initialize', dynamic_table.initialize) }} From c22bed0cf7f147ad1c939138967392610f516088 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 8 Oct 2024 15:10:15 -0700 Subject: [PATCH 5/6] Tabs. --- dbt/include/snowflake/macros/relations/dynamic_table/create.sql | 2 +- .../snowflake/macros/relations/dynamic_table/replace.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql index 4078a7dd6..4ebcf145b 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql @@ -74,7 +74,7 @@ target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} {{ optional('external_volume', dynamic_table.catalog.external_volume) }} - {{ optional('catalog', dynamic_table.catalog.name) }} + {{ optional('catalog', dynamic_table.catalog.name) }} base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} {{ optional('initialize', dynamic_table.initialize) }} diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql index bcfcffad5..2e7b4566a 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql @@ -73,7 +73,7 @@ target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} {{ optional('external_volume', dynamic_table.catalog.external_volume) }} - {{ optional('catalog', dynamic_table.catalog.name) }} + {{ optional('catalog', dynamic_table.catalog.name) }} base_location = '{{ dynamic_table.catalog.base_location }}' {{ optional('refresh_mode', dynamic_table.refresh_mode) }} {{ optional('initialize', dynamic_table.initialize) }} From 9acac4928925fbcf7a1e34dd9db74baf341d18af Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 8 Oct 2024 15:15:21 -0700 Subject: [PATCH 6/6] Fix base_location_subpath generation for dynamic tables. --- .changes/unreleased/Fixes-20241008-122635.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/Fixes-20241008-122635.yaml b/.changes/unreleased/Fixes-20241008-122635.yaml index 90cc72995..c069283d6 100644 --- a/.changes/unreleased/Fixes-20241008-122635.yaml +++ b/.changes/unreleased/Fixes-20241008-122635.yaml @@ -1,5 +1,5 @@ kind: Fixes -body: Dynamic Iceberg table DDL catalog parameter fixes. +body: Dynamic Iceberg table base_location_subpath generation fix. time: 2024-10-08T12:26:35.521308-07:00 custom: Author: versusfacit