From 9b8e04c0c0ee3f8a73123f64daa20344ee6c8c71 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 8 Oct 2024 12:23:04 -0700 Subject: [PATCH] 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: