Fix Dynamic Iceberg Table Required DDL Params (#1201)

* Fix base location not rendering without subpath and add tests. Take optional off params that are not optional in dynamic table create DDL. * Add changelog. * Revert changes to external volume * revert changes to catalog optionality. * Tabs. * Fix base_location_subpath generation for dynamic tables. --------- Co-authored-by: VersusFacit <versusfacit@users.noreply.github.com>
dbt-labs · Oct 8, 2024 · 7dea145 · 7dea145
1 parent d30670f
commit 7dea145
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 64 deletions.
diff --git a/.changes/unreleased/Fixes-20241008-122635.yaml b/.changes/unreleased/Fixes-20241008-122635.yaml
@@ -0,0 +1,6 @@
+kind: Fixes
+body: Dynamic Iceberg table base_location_subpath generation fix.
+time: 2024-10-08T12:26:35.521308-07:00
+custom:
+    Author: versusfacit
+    Issue: "1200"
diff --git 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
@@ -75,7 +75,7 @@
         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 }}
+        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
@@ -74,7 +74,7 @@
         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 }}
+        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
@@ -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
@@ -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: