From 5acb32c5842a77766ef6c8c6e1fce8b91ccecd1a Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 22 Aug 2024 01:52:33 -0700 Subject: [PATCH 01/29] Add materializations of table and dynamic table. --- dbt/adapters/snowflake/impl.py | 5 +++ .../macros/materializations/dynamic_table.sql | 1 - .../macros/relations/dynamic_table/create.sql | 28 +++++++++++---- .../relations/dynamic_table/replace.sql | 28 +++++++++++---- .../macros/relations/table/create.sql | 36 +++++++++++++++++-- 5 files changed, 80 insertions(+), 18 deletions(-) diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index 6854b199d..026f43052 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -44,6 +44,11 @@ class SnowflakeConfig(AdapterConfig): merge_update_columns: Optional[str] = None target_lag: Optional[str] = None + # extended formats + object_format: Optional[str] = None + external_volume: Optional[str] = None + base_location: Optional[str] = None + class SnowflakeAdapter(SQLAdapter): Relation = SnowflakeRelation diff --git a/dbt/include/snowflake/macros/materializations/dynamic_table.sql b/dbt/include/snowflake/macros/materializations/dynamic_table.sql index f491ef3bd..425067557 100644 --- a/dbt/include/snowflake/macros/materializations/dynamic_table.sql +++ b/dbt/include/snowflake/macros/materializations/dynamic_table.sql @@ -25,7 +25,6 @@ {% macro dynamic_table_get_build_sql(existing_relation, target_relation) %} - {% set full_refresh_mode = should_full_refresh() %} -- determine the scenario we're in: create, full_refresh, alter, refresh data diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql index 253788779..67a159e5d 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql @@ -1,16 +1,30 @@ {% macro snowflake__get_create_dynamic_table_as_sql(relation, sql) -%} {%- set dynamic_table = relation.from_config(config.model) -%} + {%- set iceberg = config.get('object_format', default='') == 'iceberg' -%} - create dynamic table {{ relation }} + {# Configure for extended Object Format #} + {% if iceberg -%} + {%- set object_format = 'iceberg' -%} + {%- else -%} + {%- set object_format = '' -%} + {%- endif -%} + + create dynamic {{ object_format }} table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - {% if dynamic_table.refresh_mode %} - refresh_mode = {{ dynamic_table.refresh_mode }} - {% endif %} - {% if dynamic_table.initialize %} - initialize = {{ dynamic_table.initialize }} - {% endif %} + {%- if iceberg %} + external_volume = {{ config.get('external_volume') }} + catalog = 'snowflake' + base_location = {{ config.get('base_location') }} + {%- else -%} + {% if dynamic_table.refresh_mode %} + refresh_mode = {{ dynamic_table.refresh_mode }} + {% endif %} + {% if dynamic_table.initialize %} + initialize = {{ dynamic_table.initialize }} + {% endif %} + {%- endif %} as ( {{ sql }} ) diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql index dbe27d66e..eda0169e7 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql @@ -1,16 +1,30 @@ {% macro snowflake__get_replace_dynamic_table_sql(relation, sql) -%} {%- set dynamic_table = relation.from_config(config.model) -%} + {%- set iceberg = config.get('object_format', default='') == 'iceberg' -%} - create or replace dynamic table {{ relation }} + {# Configure for extended Object Format #} + {% if iceberg %} + {%- set object_format = 'iceberg' -%} + {%- else -%} + {%- set object_format = '' -%} + {%- endif -%} + + create or replace dynamic {{ object_format }} table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - {% if dynamic_table.refresh_mode %} - refresh_mode = {{ dynamic_table.refresh_mode }} - {% endif %} - {% if dynamic_table.initialize %} - initialize = {{ dynamic_table.initialize }} - {% endif %} + {%- if iceberg %} + external_volume = {{ config.get('external_volume') }} + catalog = 'snowflake' + base_location = {{ config.get('base_location') }} + {%- else -%} + {% if dynamic_table.refresh_mode %} + refresh_mode = {{ dynamic_table.refresh_mode }} + {% endif %} + {% if dynamic_table.initialize %} + initialize = {{ dynamic_table.initialize }} + {% endif %} + {%- endif %} as ( {{ sql }} ) diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index c6bc8f775..006b41056 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -1,6 +1,23 @@ {% macro snowflake__create_table_as(temporary, relation, compiled_code, language='sql') -%} {%- set transient = config.get('transient', default=true) -%} + {%- set iceberg = config.get('object_format', default='') == 'iceberg' -%} + {%- if transient and iceberg -%} + {% do exceptions.raise_compiler_error("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s" % this) %} + {%- endif %} + + {%- if transient and iceberg -%} + {% do exceptions.raise_compiler_error("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s" % this) %} + {%- endif %} + + {# Configure for extended Object Format #} + {% if iceberg -%} + {%- set object_format = 'iceberg' -%} + {%- else -%} + {%- set object_format = '' -%} + {%- endif -%} + + {# Configure for plain Table materialization #} {% if temporary -%} {%- set table_type = "temporary" -%} {%- elif transient -%} @@ -9,6 +26,10 @@ {%- set table_type = "" -%} {%- endif %} + {%- set materialization_prefix = object_format or table_type -%} + {%- set alter_statement_format_prefix = object_format -%} + + {# Generate DDL/DML #} {%- if language == 'sql' -%} {%- set cluster_by_keys = config.get('cluster_by', default=none) -%} {%- set enable_automatic_clustering = config.get('automatic_clustering', default=false) -%} @@ -26,7 +47,13 @@ {{ sql_header if sql_header is not none }} - create or replace {{ table_type }} table {{ relation }} + create or replace {{ materialization_prefix }} table {{ relation }} + {%- if iceberg %} + external_volume = {{ config.get('external_volume') }} + catalog = 'snowflake' + base_location = {{ config.get('base_location') }} + {%- endif -%} + {%- set contract_config = config.get('contract') -%} {%- if contract_config.enforced -%} {{ get_assert_columns_equivalent(sql) }} @@ -44,13 +71,16 @@ {%- endif %} ); {% if cluster_by_string is not none and not temporary -%} - alter table {{relation}} cluster by ({{cluster_by_string}}); + alter {{ alter_statement_format_prefix }} table {{relation}} cluster by ({{cluster_by_string}}); {%- endif -%} {% if enable_automatic_clustering and cluster_by_string is not none and not temporary -%} - alter table {{relation}} resume recluster; + alter {{ alter_statement_format_prefix }} table {{relation}} resume recluster; {%- endif -%} {%- elif language == 'python' -%} + {%- if iceberg -%} + {% do exceptions.raise_compiler_error('Iceberg is incompatible with Python models. Please use a SQL model for the iceberg format.') %} + {%- endif %} {{ py_write_table(compiled_code=compiled_code, target_relation=relation, table_type=table_type) }} {%- else -%} {% do exceptions.raise_compiler_error("snowflake__create_table_as macro didn't get supported language, it got %s" % language) %} From c2b8d78152de03058f9187c90e99f7e0a25eb18b Mon Sep 17 00:00:00 2001 From: Mila Page Date: Mon, 26 Aug 2024 02:16:55 -0700 Subject: [PATCH 02/29] Add the method to tell something is iceberg format and pipe that through to relation object --- dbt/adapters/snowflake/impl.py | 46 +++++++++++++++++-- dbt/adapters/snowflake/relation.py | 6 +++ .../snowflake/relation_configs/__init__.py | 1 + .../snowflake/relation_configs/formats.py | 6 +++ dbt/include/snowflake/macros/adapters.sql | 7 +++ .../macros/relations/table/create.sql | 26 +++++++---- 6 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 dbt/adapters/snowflake/relation_configs/formats.py diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index 026f43052..6dcb43862 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -20,7 +20,10 @@ from dbt_common.exceptions import CompilationError, DbtDatabaseError, DbtRuntimeError from dbt_common.utils import filter_null_values -from dbt.adapters.snowflake.relation_configs import SnowflakeRelationType +from dbt.adapters.snowflake.relation_configs import ( + SnowflakeRelationType, + SnowflakeObjectFormat, +) from dbt.adapters.snowflake import SnowflakeColumn from dbt.adapters.snowflake import SnowflakeConnectionManager from dbt.adapters.snowflake import SnowflakeRelation @@ -29,6 +32,7 @@ import agate SHOW_OBJECT_METADATA_MACRO_NAME = "snowflake__show_object_metadata" +LIST_ICEBERG_RELATIONS_MACRO_NAME = "snowflake__show_iceberg_relations" @dataclass @@ -47,7 +51,7 @@ class SnowflakeConfig(AdapterConfig): # extended formats object_format: Optional[str] = None external_volume: Optional[str] = None - base_location: Optional[str] = None + base_location_subpath: Optional[str] = None class SnowflakeAdapter(SQLAdapter): @@ -228,8 +232,35 @@ def list_relations_without_caching( self, schema_relation: SnowflakeRelation ) -> List[SnowflakeRelation]: kwargs = {"schema_relation": schema_relation} + + def check_is_iceberg(row, table2): + for match_row in table2.rows: + if ( + row["name"] == match_row["name"] + and row["database_name"] == match_row["database_name"] + and row["schema_name"] == match_row["schema_name"] + ): + return "Y" + return "N" + try: - results = self.execute_macro(LIST_RELATIONS_MACRO_NAME, kwargs=kwargs) + schema_objects = self.execute_macro(LIST_RELATIONS_MACRO_NAME, kwargs=kwargs) + iceberg_table_results = self.execute_macro( + LIST_ICEBERG_RELATIONS_MACRO_NAME, kwargs=kwargs + ) + import agate + + # this only seems to only inflate runtime 16%; TODO: stress test + results = schema_objects.compute( + [ + ( + "is_iceberg", + agate.Formula( + agate.Text(), lambda row: check_is_iceberg(row, iceberg_table_results) + ), + ) + ] + ) except DbtDatabaseError as exc: # if the schema doesn't exist, we just want to return. # Alternatively, we could query the list of schemas before we start @@ -242,16 +273,19 @@ def list_relations_without_caching( columns = ["database_name", "schema_name", "name", "kind"] if "is_dynamic" in results.column_names: columns.append("is_dynamic") + if "is_iceberg" in results.column_names: + columns.append("is_iceberg") return [self._parse_list_relations_result(result) for result in results.select(columns)] def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation: # this can be reduced to always including `is_dynamic` once bundle `2024_03` is mandatory try: - database, schema, identifier, relation_type, is_dynamic = result + database, schema, identifier, relation_type, is_dynamic, is_iceberg = result except ValueError: database, schema, identifier, relation_type = result is_dynamic = "N" + is_iceberg = "N" try: relation_type = self.Relation.get_relation_type(relation_type.lower()) @@ -261,12 +295,16 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation if relation_type == self.Relation.Table and is_dynamic == "Y": relation_type = self.Relation.DynamicTable + object_format: str = ( + SnowflakeObjectFormat.ICEBERG if is_iceberg == "Y" else SnowflakeObjectFormat.DEFAULT + ) quote_policy = {"database": True, "schema": True, "identifier": True} return self.Relation.create( database=database, schema=schema, identifier=identifier, type=relation_type, + object_format=object_format, quote_policy=quote_policy, ) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index ace85695b..62660137e 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -17,6 +17,7 @@ SnowflakeDynamicTableRefreshModeConfigChange, SnowflakeDynamicTableTargetLagConfigChange, SnowflakeDynamicTableWarehouseConfigChange, + SnowflakeObjectFormat, SnowflakeQuotePolicy, SnowflakeRelationType, ) @@ -25,6 +26,7 @@ @dataclass(frozen=True, eq=False, repr=False) class SnowflakeRelation(BaseRelation): type: Optional[SnowflakeRelationType] = None + object_format: str = SnowflakeObjectFormat.DEFAULT quote_policy: SnowflakeQuotePolicy = field(default_factory=lambda: SnowflakeQuotePolicy()) require_alias: bool = False relation_configs = { @@ -53,6 +55,10 @@ class SnowflakeRelation(BaseRelation): def is_dynamic_table(self) -> bool: return self.type == SnowflakeRelationType.DynamicTable + @property + def is_iceberg_format(self) -> bool: + return self.object_format == SnowflakeRelationType.ICEBERG + @classproperty def DynamicTable(cls) -> str: return str(SnowflakeRelationType.DynamicTable) diff --git a/dbt/adapters/snowflake/relation_configs/__init__.py b/dbt/adapters/snowflake/relation_configs/__init__.py index 62f95faff..f2b3c85dc 100644 --- a/dbt/adapters/snowflake/relation_configs/__init__.py +++ b/dbt/adapters/snowflake/relation_configs/__init__.py @@ -10,3 +10,4 @@ SnowflakeQuotePolicy, SnowflakeRelationType, ) +from dbt.adapters.snowflake.relation_configs.formats import SnowflakeObjectFormat diff --git a/dbt/adapters/snowflake/relation_configs/formats.py b/dbt/adapters/snowflake/relation_configs/formats.py new file mode 100644 index 000000000..c5d1afc3a --- /dev/null +++ b/dbt/adapters/snowflake/relation_configs/formats.py @@ -0,0 +1,6 @@ +from dbt_common.dataclass_schema import StrEnum # doesn't exist in standard library until py3.11 + + +class SnowflakeObjectFormat(StrEnum): + DEFAULT = "default" + ICEBERG = "iceberg" diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index 4cb4bcffa..62408a286 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -318,3 +318,10 @@ {{ snowflake_dml_explicit_transaction(truncate_dml) }} {%- endcall %} {% endmacro %} + + +{% macro snowflake__show_iceberg_relations(schema_relation) %} + {%- set sql = 'show objects in ' ~ schema_relation ~ ';' %} + {%- set result = run_query(sql) -%} + {%- do return(result) -%} +{% endmacro %} diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 006b41056..0cc305173 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -10,12 +10,6 @@ {% do exceptions.raise_compiler_error("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s" % this) %} {%- endif %} - {# Configure for extended Object Format #} - {% if iceberg -%} - {%- set object_format = 'iceberg' -%} - {%- else -%} - {%- set object_format = '' -%} - {%- endif -%} {# Configure for plain Table materialization #} {% if temporary -%} @@ -26,6 +20,7 @@ {%- set table_type = "" -%} {%- endif %} + {%- set object_format = 'iceberg' -%} {%- set materialization_prefix = object_format or table_type -%} {%- set alter_statement_format_prefix = object_format -%} @@ -49,9 +44,11 @@ create or replace {{ materialization_prefix }} table {{ relation }} {%- if iceberg %} - external_volume = {{ config.get('external_volume') }} - catalog = 'snowflake' - base_location = {{ config.get('base_location') }} + {# + Valid DDL in CTAS statements. Plain create statements have a different order. + https://docs.snowflake.com/en/sql-reference/sql/create-iceberg-table + #} + {{ render_iceberg_ddl(relation) }} {%- endif -%} {%- set contract_config = config.get('contract') -%} @@ -87,3 +84,14 @@ {%- endif -%} {% endmacro %} + +{% macro render_iceberg_ddl(relation) -%} + {%- set external_volume = config.get('external_volume') -%} + {# S3 treats subpaths with or without a trailing '/' as functionally equivalent #} + {%- set subpath = config.get('base_location_subpath') -%} + {%- set base_location = '_dbt/' ~ relation.schema ~ '/' ~ relation.name ~ (('/' ~ subpath) if subpath else '') -%} + + external_volume = '{{ external_volume }}' + catalog = 'snowflake' + base_location = '{{ base_location }}' +{% endmacro %} From 5afb5511d8c6f8e4999858a3cd40156be72caec8 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 28 Aug 2024 20:58:30 -0700 Subject: [PATCH 03/29] Finish create macro and fix alters. --- .../macros/relations/table/create.sql | 97 +++++++++++++------ 1 file changed, 69 insertions(+), 28 deletions(-) diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 0cc305173..56a500c6b 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -1,28 +1,6 @@ {% macro snowflake__create_table_as(temporary, relation, compiled_code, language='sql') -%} - {%- set transient = config.get('transient', default=true) -%} - {%- set iceberg = config.get('object_format', default='') == 'iceberg' -%} - - {%- if transient and iceberg -%} - {% do exceptions.raise_compiler_error("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s" % this) %} - {%- endif %} - - {%- if transient and iceberg -%} - {% do exceptions.raise_compiler_error("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s" % this) %} - {%- endif %} - - - {# Configure for plain Table materialization #} - {% if temporary -%} - {%- set table_type = "temporary" -%} - {%- elif transient -%} - {%- set table_type = "transient" -%} - {%- else -%} - {%- set table_type = "" -%} - {%- endif %} - - {%- set object_format = 'iceberg' -%} - {%- set materialization_prefix = object_format or table_type -%} - {%- set alter_statement_format_prefix = object_format -%} + {%- set materialization_prefix = get_create_ddl_prefix(temporary) -%} + {%- set alter_prefix = get_alter_ddl_prefix() -%} {# Generate DDL/DML #} {%- if language == 'sql' -%} @@ -43,12 +21,13 @@ {{ sql_header if sql_header is not none }} create or replace {{ materialization_prefix }} table {{ relation }} - {%- if iceberg %} + {%- if _is_iceberg_relation() %} {# Valid DDL in CTAS statements. Plain create statements have a different order. https://docs.snowflake.com/en/sql-reference/sql/create-iceberg-table #} {{ render_iceberg_ddl(relation) }} + {% else %} {%- endif -%} {%- set contract_config = config.get('contract') -%} @@ -68,10 +47,10 @@ {%- endif %} ); {% if cluster_by_string is not none and not temporary -%} - alter {{ alter_statement_format_prefix }} table {{relation}} cluster by ({{cluster_by_string}}); + alter {{ alter_prefix }} table {{relation}} cluster by ({{cluster_by_string}}); {%- endif -%} - {% if enable_automatic_clustering and cluster_by_string is not none and not temporary -%} - alter {{ alter_statement_format_prefix }} table {{relation}} resume recluster; + {% if enable_automatic_clustering and cluster_by_string is not none and not temporary %} + alter {{ alter_prefix }} table {{relation}} resume recluster; {%- endif -%} {%- elif language == 'python' -%} @@ -85,6 +64,68 @@ {% endmacro %} + +{# + # Helper Macros + #} + +{% macro get_create_ddl_prefix(temporary) %} + {# + This macro generates the appropriate DDL prefix for creating a table in Snowflake, + considering the mutually exclusive nature of certain table types: + + - ICEBERG: A specific storage format that requires a distinct DDL layout. + - TEMPORARY: Indicates a table that exists only for the duration of the session. + - TRANSIENT: A type of table that is similar to a permanent table but without fail-safe. + + Note: If ICEBERG is specified, transient=true throws a warning because ICEBERG + does not support transient tables. + #} + + {%- set is_iceberg = _is_iceberg_relation() -%} + {%- set is_temporary = temporary -%} + {%- set is_transient = config.get('transient', default=true) -%} + + {%- if is_transient and is_iceberg -%} + {{ exceptions.warn("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s. If left unmodified, dbt will ignore 'transient'." % this) }} + {%- endif %} + + {%- if is_temporary and is_iceberg -%} + {{ exceptions.warn("Iceberg format relations cannot be temporary. Please remove either the transient or iceberg parameters from %s. If left unmodified, dbt will ignore 'transient'." % this) }} + {%- endif %} + + {%- if is_iceberg -%} + {{ return('iceberg') }} + {%- elif is_temporary -%} + {{ return('temporary') }} + {%- elif is_transient -%} + {{ return('transient') }} + {%- else -%} + {{ return('') }} + {%- endif -%} +{% endmacro %} + + +{% macro get_alter_ddl_prefix() %} + {# All ALTER statements on Iceberg tables require an ICEBERG prefix #} + {%- if _get_relation_object_format() == 'iceberg' -%} + {{ return('iceberg') }} + {%- else -%} + {{ return('') }} + {%- endif -%} +{% endmacro %} + + +{% macro _get_relation_object_format() %} + {{ return(config.get('object_format', default='')) }} +{% endmacro %} + + +{% macro _is_iceberg_relation() %} + {{ return(_get_relation_object_format() == 'iceberg') }} +{% endmacro %} + + {% macro render_iceberg_ddl(relation) -%} {%- set external_volume = config.get('external_volume') -%} {# S3 treats subpaths with or without a trailing '/' as functionally equivalent #} From 53eb5b958d9ed1e6683d052d9a1acaac5aa215e9 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 28 Aug 2024 22:06:59 -0700 Subject: [PATCH 04/29] Finish todo items and begin cleaning code. --- dbt/adapters/snowflake/relation.py | 2 +- .../snowflake/relation_configs/formats.py | 3 ++ .../macros/materializations/table.sql | 36 +++++++++++++++---- .../macros/relations/table/create.sql | 2 +- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 62660137e..856daa09d 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -57,7 +57,7 @@ def is_dynamic_table(self) -> bool: @property def is_iceberg_format(self) -> bool: - return self.object_format == SnowflakeRelationType.ICEBERG + return self.object_format == SnowflakeObjectFormat.ICEBERG @classproperty def DynamicTable(cls) -> str: diff --git a/dbt/adapters/snowflake/relation_configs/formats.py b/dbt/adapters/snowflake/relation_configs/formats.py index c5d1afc3a..05ed50c5d 100644 --- a/dbt/adapters/snowflake/relation_configs/formats.py +++ b/dbt/adapters/snowflake/relation_configs/formats.py @@ -4,3 +4,6 @@ class SnowflakeObjectFormat(StrEnum): DEFAULT = "default" ICEBERG = "iceberg" + + def __str__(self): + return self.value diff --git a/dbt/include/snowflake/macros/materializations/table.sql b/dbt/include/snowflake/macros/materializations/table.sql index ef201c705..32a312a42 100644 --- a/dbt/include/snowflake/macros/materializations/table.sql +++ b/dbt/include/snowflake/macros/materializations/table.sql @@ -14,12 +14,7 @@ {{ run_hooks(pre_hooks) }} - {#-- Drop the relation if it was a view to "convert" it in a table. This may lead to - -- downtime, but it should be a relatively infrequent occurrence #} - {% if old_relation is not none and not old_relation.is_table %} - {{ log("Dropping relation " ~ old_relation ~ " because it is of type " ~ old_relation.type) }} - {{ drop_relation_if_exists(old_relation) }} - {% endif %} + {{ drop_old_relation_as_needed(old_relation, target_relation) }} {% call statement('main', language=language) -%} {{ create_table_as(False, target_relation, compiled_code, language) }} @@ -85,3 +80,32 @@ def main(session): # dbt = dbtObj(session.table) # df = model(dbt, session) {%endmacro%} + + +{% macro drop_old_relation_as_needed(old_relation, target_relation) %} + {# + -- Each of these will cause some latency, but it shoudl be a relatively infrequent occurrence. + + -- An existing view must be dropped for model to "converT" into a table" + #} + {% if old_relation is not none and not old_relation.is_table %} + {{ log("Dropping relation " ~ old_relation ~ " because it is of type " ~ old_relation.type) }} + {{ drop_relation_if_exists(old_relation) }} + {% endif %} + + {# + -- An existing Iceberg table must be dropped for model to "convert" into a table. + #} + {% if old_relation is not none and old_relation.is_iceberg_format %} + {{ log("Dropping relation " ~ old_relation ~ " because it is an Iceberg format table " ~ old_relation.object_format) }} + {{ drop_relation_if_exists(old_relation) }} + {% endif %} + + {# + -- An existing table must be dropped for model to "convert" into an Iceberg table. + #} + {% if old_relation is not none and old_relation.is_table and target_relation.is_iceberg_format %} + {{ log("Dropping relation " ~ old_relation ~ " because it is a table and target relation is Iceberg format") }} + {{ drop_relation_if_exists(old_relation) }} + {% endif %} +{% endmacro %} diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 56a500c6b..12ee13523 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -84,7 +84,7 @@ {%- set is_iceberg = _is_iceberg_relation() -%} {%- set is_temporary = temporary -%} - {%- set is_transient = config.get('transient', default=true) -%} + {%- set is_transient = config.get('transient', default=False) -%} {%- if is_transient and is_iceberg -%} {{ exceptions.warn("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s. If left unmodified, dbt will ignore 'transient'." % this) }} From a3b13b85fa290c90bd8da9b1177adc729284a8c2 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 28 Aug 2024 22:10:43 -0700 Subject: [PATCH 05/29] revert dynamic table changes. --- .../macros/materializations/dynamic_table.sql | 1 + .../macros/relations/dynamic_table/create.sql | 28 +++++-------------- .../relations/dynamic_table/replace.sql | 28 +++++-------------- 3 files changed, 15 insertions(+), 42 deletions(-) diff --git a/dbt/include/snowflake/macros/materializations/dynamic_table.sql b/dbt/include/snowflake/macros/materializations/dynamic_table.sql index 425067557..f491ef3bd 100644 --- a/dbt/include/snowflake/macros/materializations/dynamic_table.sql +++ b/dbt/include/snowflake/macros/materializations/dynamic_table.sql @@ -25,6 +25,7 @@ {% macro dynamic_table_get_build_sql(existing_relation, target_relation) %} + {% set full_refresh_mode = should_full_refresh() %} -- determine the scenario we're in: create, full_refresh, alter, refresh data diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql index 67a159e5d..253788779 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/create.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/create.sql @@ -1,30 +1,16 @@ {% macro snowflake__get_create_dynamic_table_as_sql(relation, sql) -%} {%- set dynamic_table = relation.from_config(config.model) -%} - {%- set iceberg = config.get('object_format', default='') == 'iceberg' -%} - {# Configure for extended Object Format #} - {% if iceberg -%} - {%- set object_format = 'iceberg' -%} - {%- else -%} - {%- set object_format = '' -%} - {%- endif -%} - - create dynamic {{ object_format }} table {{ relation }} + create dynamic table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - {%- if iceberg %} - external_volume = {{ config.get('external_volume') }} - catalog = 'snowflake' - base_location = {{ config.get('base_location') }} - {%- else -%} - {% if dynamic_table.refresh_mode %} - refresh_mode = {{ dynamic_table.refresh_mode }} - {% endif %} - {% if dynamic_table.initialize %} - initialize = {{ dynamic_table.initialize }} - {% endif %} - {%- endif %} + {% if dynamic_table.refresh_mode %} + refresh_mode = {{ dynamic_table.refresh_mode }} + {% endif %} + {% if dynamic_table.initialize %} + initialize = {{ dynamic_table.initialize }} + {% endif %} as ( {{ sql }} ) diff --git a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql index eda0169e7..dbe27d66e 100644 --- a/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql +++ b/dbt/include/snowflake/macros/relations/dynamic_table/replace.sql @@ -1,30 +1,16 @@ {% macro snowflake__get_replace_dynamic_table_sql(relation, sql) -%} {%- set dynamic_table = relation.from_config(config.model) -%} - {%- set iceberg = config.get('object_format', default='') == 'iceberg' -%} - {# Configure for extended Object Format #} - {% if iceberg %} - {%- set object_format = 'iceberg' -%} - {%- else -%} - {%- set object_format = '' -%} - {%- endif -%} - - create or replace dynamic {{ object_format }} table {{ relation }} + create or replace dynamic table {{ relation }} target_lag = '{{ dynamic_table.target_lag }}' warehouse = {{ dynamic_table.snowflake_warehouse }} - {%- if iceberg %} - external_volume = {{ config.get('external_volume') }} - catalog = 'snowflake' - base_location = {{ config.get('base_location') }} - {%- else -%} - {% if dynamic_table.refresh_mode %} - refresh_mode = {{ dynamic_table.refresh_mode }} - {% endif %} - {% if dynamic_table.initialize %} - initialize = {{ dynamic_table.initialize }} - {% endif %} - {%- endif %} + {% if dynamic_table.refresh_mode %} + refresh_mode = {{ dynamic_table.refresh_mode }} + {% endif %} + {% if dynamic_table.initialize %} + initialize = {{ dynamic_table.initialize }} + {% endif %} as ( {{ sql }} ) From 37006aea6252ab4a2cb5d9f17a49c9d7951c2b2a Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 28 Aug 2024 22:47:26 -0700 Subject: [PATCH 06/29] Fix the drop by fixing snowflake__show_iceberg_relations --- dbt/include/snowflake/macros/adapters.sql | 2 +- .../macros/materializations/table.sql | 28 +++++++++++-------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index 62408a286..d9d9d8753 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -321,7 +321,7 @@ {% macro snowflake__show_iceberg_relations(schema_relation) %} - {%- set sql = 'show objects in ' ~ schema_relation ~ ';' %} + {%- set sql = 'show iceberg tables in ' ~ schema_relation ~ ';' %} {%- set result = run_query(sql) -%} {%- do return(result) -%} {% endmacro %} diff --git a/dbt/include/snowflake/macros/materializations/table.sql b/dbt/include/snowflake/macros/materializations/table.sql index 32a312a42..cbb691da6 100644 --- a/dbt/include/snowflake/macros/materializations/table.sql +++ b/dbt/include/snowflake/macros/materializations/table.sql @@ -8,9 +8,13 @@ {% set grant_config = config.get('grants') %} {%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%} - {%- set target_relation = api.Relation.create(identifier=identifier, - schema=schema, - database=database, type='table') -%} + {%- set target_relation = api.Relation.create( + identifier=identifier, + schema=schema, + database=database, + type='table', + object_format=config.get('object_format', 'default') + ) -%} {{ run_hooks(pre_hooks) }} @@ -83,29 +87,31 @@ def main(session): {% macro drop_old_relation_as_needed(old_relation, target_relation) %} + {% if old_relation is none %} + {{ return('') }} + {% endif %} + {# -- Each of these will cause some latency, but it shoudl be a relatively infrequent occurrence. - -- An existing view must be dropped for model to "converT" into a table" + -- An existing view must be dropped for model to "convert" into a table" #} - {% if old_relation is not none and not old_relation.is_table %} + {% if not old_relation.is_table %} {{ log("Dropping relation " ~ old_relation ~ " because it is of type " ~ old_relation.type) }} {{ drop_relation_if_exists(old_relation) }} - {% endif %} {# -- An existing Iceberg table must be dropped for model to "convert" into a table. #} - {% if old_relation is not none and old_relation.is_iceberg_format %} - {{ log("Dropping relation " ~ old_relation ~ " because it is an Iceberg format table " ~ old_relation.object_format) }} + {% elif old_relation.is_iceberg_format and not target_relation.is_iceberg_format %} + {{ log("Dropping relation " ~ old_relation ~ " because it is an Iceberg format table and target relation " ~ target_relation ~ " is a default format table.") }} {{ drop_relation_if_exists(old_relation) }} - {% endif %} {# -- An existing table must be dropped for model to "convert" into an Iceberg table. #} - {% if old_relation is not none and old_relation.is_table and target_relation.is_iceberg_format %} - {{ log("Dropping relation " ~ old_relation ~ " because it is a table and target relation is Iceberg format") }} + {% elif old_relation.is_table and not old_relation.is_iceberg_format and target_relation.is_iceberg_format %} + {{ log("Dropping relation " ~ old_relation ~ " because it is a default format table and target relation is an Iceberg format table.") }} {{ drop_relation_if_exists(old_relation) }} {% endif %} {% endmacro %} From 188720860f09ce1850ccd956d80346086cdbf4ba Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 28 Aug 2024 23:29:31 -0700 Subject: [PATCH 07/29] Transient needs sophisticated handling based on what user specifies for transient manually. --- dbt/include/snowflake/macros/adapters.sql | 4 +++- .../macros/relations/table/create.sql | 23 +++++++++++-------- .../list_relations_tests/test_show_objects.py | 2 +- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index d9d9d8753..1662b46a2 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -321,7 +321,9 @@ {% macro snowflake__show_iceberg_relations(schema_relation) %} - {%- set sql = 'show iceberg tables in ' ~ schema_relation ~ ';' %} + {%- set sql -%} + show iceberg tables in {{ schema_relation }} + {%- endset -%} {%- set result = run_query(sql) -%} {%- do return(result) -%} {% endmacro %} diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 12ee13523..03107c7a1 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -84,22 +84,27 @@ {%- set is_iceberg = _is_iceberg_relation() -%} {%- set is_temporary = temporary -%} - {%- set is_transient = config.get('transient', default=False) -%} - {%- if is_transient and is_iceberg -%} - {{ exceptions.warn("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s. If left unmodified, dbt will ignore 'transient'." % this) }} - {%- endif %} + {%- if is_iceberg -%} + {# -- Check if user supplied a transient model config of True. #} + {%- if config.get('transient') == True -%} + {{ exceptions.warn("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s. If left unmodified, dbt will ignore 'transient'." % this) }} + {%- endif %} - {%- if is_temporary and is_iceberg -%} - {{ exceptions.warn("Iceberg format relations cannot be temporary. Please remove either the transient or iceberg parameters from %s. If left unmodified, dbt will ignore 'transient'." % this) }} - {%- endif %} + {# -- Check if runtime is trying to create a Temporary Iceberg table. #} + {%- if is_temporary -%} + {{ exceptions.raise_compiler_error("Iceberg format relations cannot be temporary. Temporary is being inserted into an Iceberg format table while materializing %s." % this) }} + {%- endif %} - {%- if is_iceberg -%} {{ return('iceberg') }} + {%- elif is_temporary -%} {{ return('temporary') }} - {%- elif is_transient -%} + + {# -- Always supply transient on table create DDL unless user specifically sets transient to false. #} + {%- elif config.get('transient', default=true) -%} {{ return('transient') }} + {%- else -%} {{ return('') }} {%- endif -%} diff --git a/tests/functional/adapter/list_relations_tests/test_show_objects.py b/tests/functional/adapter/list_relations_tests/test_show_objects.py index e5eee39d9..f59dc335b 100644 --- a/tests/functional/adapter/list_relations_tests/test_show_objects.py +++ b/tests/functional/adapter/list_relations_tests/test_show_objects.py @@ -73,7 +73,7 @@ def list_relations_without_caching(project) -> List[SnowflakeRelation]: database=project.database, schema=project.test_schema, identifier="" ) with get_connection(my_adapter): - relations = my_adapter.list_relations_without_caching(schema) + relations = my_adapter.list_relations_without_caching(schema.path.schema) return relations def test_list_relations_without_caching(self, project): From 815026173152f510b4f44c0f7e3b790edf6d8fa3 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 29 Aug 2024 00:24:03 -0700 Subject: [PATCH 08/29] Try to figure out what the right None semantics are. --- dbt/include/snowflake/macros/relations/table/create.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 03107c7a1..05d1c5d75 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -101,8 +101,8 @@ {%- elif is_temporary -%} {{ return('temporary') }} - {# -- Always supply transient on table create DDL unless user specifically sets transient to false. #} - {%- elif config.get('transient', default=true) -%} + {# -- Always supply transient on table create DDL unless user specifically sets transient to false or None. #} + {%- elif config.get('transient') is not defined or config.get('transient') == True -%} {{ return('transient') }} {%- else -%} From 74ec1a38e82b738fa347fdc1b5b2cff42b7d1c0b Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 29 Aug 2024 00:44:11 -0700 Subject: [PATCH 09/29] Revert to original statement. --- dbt/include/snowflake/macros/relations/table/create.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 05d1c5d75..403af211b 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -102,7 +102,7 @@ {{ return('temporary') }} {# -- Always supply transient on table create DDL unless user specifically sets transient to false or None. #} - {%- elif config.get('transient') is not defined or config.get('transient') == True -%} + {%- elif config.get('transient', default=True) -%} {{ return('transient') }} {%- else -%} From 229722699c07ab80ba41cd3fea190ba63b521447 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Thu, 29 Aug 2024 01:09:09 -0700 Subject: [PATCH 10/29] Fix the transient behavior by passing table_type again. --- dbt/include/snowflake/macros/relations/table/create.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 403af211b..472b11079 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -57,7 +57,7 @@ {%- if iceberg -%} {% do exceptions.raise_compiler_error('Iceberg is incompatible with Python models. Please use a SQL model for the iceberg format.') %} {%- endif %} - {{ py_write_table(compiled_code=compiled_code, target_relation=relation, table_type=table_type) }} + {{ py_write_table(compiled_code=compiled_code, target_relation=relation, table_type=get_create_ddl_prefix(temporary)) }} {%- else -%} {% do exceptions.raise_compiler_error("snowflake__create_table_as macro didn't get supported language, it got %s" % language) %} {%- endif -%} @@ -102,7 +102,7 @@ {{ return('temporary') }} {# -- Always supply transient on table create DDL unless user specifically sets transient to false or None. #} - {%- elif config.get('transient', default=True) -%} + {%- elif config.get('transient', default=true) -%} {{ return('transient') }} {%- else -%} From 1c26ee3fd1219f06998d87a3a5115ae68ff4bdb9 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 10 Sep 2024 15:03:28 -0700 Subject: [PATCH 11/29] Rename object_format config param to table_format --- dbt/adapters/snowflake/impl.py | 6 +++--- dbt/adapters/snowflake/relation.py | 4 ++-- dbt/include/snowflake/macros/materializations/table.sql | 2 +- dbt/include/snowflake/macros/relations/table/create.sql | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index 6dcb43862..2560e27e2 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -49,7 +49,7 @@ class SnowflakeConfig(AdapterConfig): target_lag: Optional[str] = None # extended formats - object_format: Optional[str] = None + table_format: Optional[str] = None external_volume: Optional[str] = None base_location_subpath: Optional[str] = None @@ -295,7 +295,7 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation if relation_type == self.Relation.Table and is_dynamic == "Y": relation_type = self.Relation.DynamicTable - object_format: str = ( + table_format: str = ( SnowflakeObjectFormat.ICEBERG if is_iceberg == "Y" else SnowflakeObjectFormat.DEFAULT ) quote_policy = {"database": True, "schema": True, "identifier": True} @@ -304,7 +304,7 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation schema=schema, identifier=identifier, type=relation_type, - object_format=object_format, + table_format=table_format, quote_policy=quote_policy, ) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 856daa09d..102db1206 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -26,7 +26,7 @@ @dataclass(frozen=True, eq=False, repr=False) class SnowflakeRelation(BaseRelation): type: Optional[SnowflakeRelationType] = None - object_format: str = SnowflakeObjectFormat.DEFAULT + table_format: str = SnowflakeObjectFormat.DEFAULT quote_policy: SnowflakeQuotePolicy = field(default_factory=lambda: SnowflakeQuotePolicy()) require_alias: bool = False relation_configs = { @@ -57,7 +57,7 @@ def is_dynamic_table(self) -> bool: @property def is_iceberg_format(self) -> bool: - return self.object_format == SnowflakeObjectFormat.ICEBERG + return self.table_format == SnowflakeObjectFormat.ICEBERG @classproperty def DynamicTable(cls) -> str: diff --git a/dbt/include/snowflake/macros/materializations/table.sql b/dbt/include/snowflake/macros/materializations/table.sql index cbb691da6..9cadeff0f 100644 --- a/dbt/include/snowflake/macros/materializations/table.sql +++ b/dbt/include/snowflake/macros/materializations/table.sql @@ -13,7 +13,7 @@ schema=schema, database=database, type='table', - object_format=config.get('object_format', 'default') + table_format=config.get('table_format', 'default') ) -%} {{ run_hooks(pre_hooks) }} diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 472b11079..976b660b5 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -113,7 +113,7 @@ {% macro get_alter_ddl_prefix() %} {# All ALTER statements on Iceberg tables require an ICEBERG prefix #} - {%- if _get_relation_object_format() == 'iceberg' -%} + {%- if _get_relation_table_format() == 'iceberg' -%} {{ return('iceberg') }} {%- else -%} {{ return('') }} @@ -121,13 +121,13 @@ {% endmacro %} -{% macro _get_relation_object_format() %} - {{ return(config.get('object_format', default='')) }} +{% macro _get_relation_table_format() %} + {{ return(config.get('table_format', default='')) }} {% endmacro %} {% macro _is_iceberg_relation() %} - {{ return(_get_relation_object_format() == 'iceberg') }} + {{ return(_get_relation_table_format() == 'iceberg') }} {% endmacro %} From 491a76a1120bb50785de6fd8edc1312a947477b8 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 10 Sep 2024 17:43:04 -0700 Subject: [PATCH 12/29] Migrate Jinja macros to Python. --- dbt/adapters/snowflake/impl.py | 1 + dbt/adapters/snowflake/relation.py | 76 +++++++++++++++- .../macros/relations/table/create.sql | 86 +------------------ 3 files changed, 80 insertions(+), 83 deletions(-) diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index 2560e27e2..eb1443809 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -299,6 +299,7 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation SnowflakeObjectFormat.ICEBERG if is_iceberg == "Y" else SnowflakeObjectFormat.DEFAULT ) quote_policy = {"database": True, "schema": True, "identifier": True} + return self.Relation.create( database=database, schema=schema, diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 102db1206..fd27ee7c8 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -1,8 +1,12 @@ +import textwrap + from dataclasses import dataclass, field -from typing import FrozenSet, Optional, Type +from typing import FrozenSet, Optional, Type, TYPE_CHECKING + from dbt.adapters.base.relation import BaseRelation from dbt.adapters.contracts.relation import ComponentName, RelationConfig +from dbt.adapters.events.types import AdapterEventWarning from dbt.adapters.relation_configs import ( RelationConfigBase, RelationConfigChangeAction, @@ -10,6 +14,7 @@ ) from dbt.adapters.utils import classproperty from dbt_common.exceptions import DbtRuntimeError +from dbt_common.events.functions import warn_or_error from dbt.adapters.snowflake.relation_configs import ( SnowflakeDynamicTableConfig, @@ -22,9 +27,13 @@ SnowflakeRelationType, ) +if TYPE_CHECKING: + from dbt.artifacts.resources.v1.model import ModelConfig + @dataclass(frozen=True, eq=False, repr=False) class SnowflakeRelation(BaseRelation): + transient: Optional[bool] = None type: Optional[SnowflakeRelationType] = None table_format: str = SnowflakeObjectFormat.DEFAULT quote_policy: SnowflakeQuotePolicy = field(default_factory=lambda: SnowflakeQuotePolicy()) @@ -126,3 +135,68 @@ def as_case_sensitive(self) -> "SnowflakeRelation": path_part_map[path] = part.upper() return self.replace_path(**path_part_map) + + def get_ddl_prefix_for_create(self, config: "ModelConfig", temporary: bool): + """ + This macro renders the appropriate DDL prefix during the create_table_as + macro. It decides based on mutually exclusive table configuration options: + + - TEMPORARY: Indicates a table that exists only for the duration of the session. + - ICEBERG: A specific storage format that requires a distinct DDL layout. + - TRANSIENT: A table similar to a permanent table but without fail-safe. + + Additional Caveats for Iceberg models: + - transient=true throws a warning because Iceberg does not support transient tables + - A temporary relation is never an Iceberg relation because Iceberg does not + support temporary relations. + """ + + transient_explicitly_set_true: bool = config.get("transient", False) + + # Temporary tables are a Snowflake feature that do not exist in the + # Iceberg framework. We ignore the Iceberg status of the model. + if temporary: + return "temporary" + elif self.is_iceberg_format: + # Log a warning that transient=true on an Iceberg relation is ignored. + if transient_explicitly_set_true: + warn_or_error( + AdapterEventWarning( + base_msg=( + "Iceberg format relations cannot be transient. Please " + "remove either the transient or iceberg config options " + f"from {self.path.database}.{self.path.schema}." + f"{self.path.identifier}. If left unmodified, dbt will " + "ignore 'transient'." + ) + ) + ) + + return "iceberg" + + # Always supply transient on table create DDL unless user specifically sets + # transient to false or unset. + elif transient_explicitly_set_true or config.get("transient") is None: + return "transient" + else: + return "" + + def get_ddl_prefix_for_alter(self): + """All ALTER statements on Iceberg tables require an ICEBERG prefix""" + if self.is_iceberg_format: + return "iceberg" + else: + return "" + + def render_iceberg_ddl(self, config: "ModelConfig"): + base_location: str = f"_dbt/{self.schema}/{self.name}" + + if subpath := config.get("base_location_subpath"): + base_location += f"/{subpath}" + + iceberg_ddl_predicates: str = f""" + external_volume = '{config.get('external_volume')}' + catalog = 'snowflake' + base_location = '{base_location}' + """ + return textwrap.indent(textwrap.dedent(iceberg_ddl_predicates), " " * 10) diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 976b660b5..f82a40277 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -1,6 +1,6 @@ {% macro snowflake__create_table_as(temporary, relation, compiled_code, language='sql') -%} - {%- set materialization_prefix = get_create_ddl_prefix(temporary) -%} - {%- set alter_prefix = get_alter_ddl_prefix() -%} + {%- set materialization_prefix = relation.get_ddl_prefix_for_create(config.model.config, temporary) -%} + {%- set alter_prefix = relation.get_ddl_prefix_for_alter() -%} {# Generate DDL/DML #} {%- if language == 'sql' -%} @@ -21,12 +21,12 @@ {{ sql_header if sql_header is not none }} create or replace {{ materialization_prefix }} table {{ relation }} - {%- if _is_iceberg_relation() %} + {%- if relation.is_iceberg_format %} {# Valid DDL in CTAS statements. Plain create statements have a different order. https://docs.snowflake.com/en/sql-reference/sql/create-iceberg-table #} - {{ render_iceberg_ddl(relation) }} + {{ relation.render_iceberg_ddl(config.model.config) }} {% else %} {%- endif -%} @@ -63,81 +63,3 @@ {%- endif -%} {% endmacro %} - - -{# - # Helper Macros - #} - -{% macro get_create_ddl_prefix(temporary) %} - {# - This macro generates the appropriate DDL prefix for creating a table in Snowflake, - considering the mutually exclusive nature of certain table types: - - - ICEBERG: A specific storage format that requires a distinct DDL layout. - - TEMPORARY: Indicates a table that exists only for the duration of the session. - - TRANSIENT: A type of table that is similar to a permanent table but without fail-safe. - - Note: If ICEBERG is specified, transient=true throws a warning because ICEBERG - does not support transient tables. - #} - - {%- set is_iceberg = _is_iceberg_relation() -%} - {%- set is_temporary = temporary -%} - - {%- if is_iceberg -%} - {# -- Check if user supplied a transient model config of True. #} - {%- if config.get('transient') == True -%} - {{ exceptions.warn("Iceberg format relations cannot be transient. Please remove either the transient or iceberg parameters from %s. If left unmodified, dbt will ignore 'transient'." % this) }} - {%- endif %} - - {# -- Check if runtime is trying to create a Temporary Iceberg table. #} - {%- if is_temporary -%} - {{ exceptions.raise_compiler_error("Iceberg format relations cannot be temporary. Temporary is being inserted into an Iceberg format table while materializing %s." % this) }} - {%- endif %} - - {{ return('iceberg') }} - - {%- elif is_temporary -%} - {{ return('temporary') }} - - {# -- Always supply transient on table create DDL unless user specifically sets transient to false or None. #} - {%- elif config.get('transient', default=true) -%} - {{ return('transient') }} - - {%- else -%} - {{ return('') }} - {%- endif -%} -{% endmacro %} - - -{% macro get_alter_ddl_prefix() %} - {# All ALTER statements on Iceberg tables require an ICEBERG prefix #} - {%- if _get_relation_table_format() == 'iceberg' -%} - {{ return('iceberg') }} - {%- else -%} - {{ return('') }} - {%- endif -%} -{% endmacro %} - - -{% macro _get_relation_table_format() %} - {{ return(config.get('table_format', default='')) }} -{% endmacro %} - - -{% macro _is_iceberg_relation() %} - {{ return(_get_relation_table_format() == 'iceberg') }} -{% endmacro %} - - -{% macro render_iceberg_ddl(relation) -%} - {%- set external_volume = config.get('external_volume') -%} - {# S3 treats subpaths with or without a trailing '/' as functionally equivalent #} - {%- set subpath = config.get('base_location_subpath') -%} - {%- set base_location = '_dbt/' ~ relation.schema ~ '/' ~ relation.name ~ (('/' ~ subpath) if subpath else '') -%} - - external_volume = '{{ external_volume }}' - catalog = 'snowflake' - base_location = '{{ base_location }}' -{% endmacro %} From c7192d3463e30aa267d92ce7d51bc106c8d21dd7 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 10 Sep 2024 17:58:19 -0700 Subject: [PATCH 13/29] All classes are frozen --- dbt/adapters/snowflake/relation.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index fd27ee7c8..6a50fab88 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -33,7 +33,6 @@ @dataclass(frozen=True, eq=False, repr=False) class SnowflakeRelation(BaseRelation): - transient: Optional[bool] = None type: Optional[SnowflakeRelationType] = None table_format: str = SnowflakeObjectFormat.DEFAULT quote_policy: SnowflakeQuotePolicy = field(default_factory=lambda: SnowflakeQuotePolicy()) @@ -175,7 +174,7 @@ def get_ddl_prefix_for_create(self, config: "ModelConfig", temporary: bool): return "iceberg" # Always supply transient on table create DDL unless user specifically sets - # transient to false or unset. + # transient to false or unset. Might as well update the object attribute too! elif transient_explicitly_set_true or config.get("transient") is None: return "transient" else: From 6d77f698396609ba30464adb87311774be5c8122 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Tue, 10 Sep 2024 23:58:53 -0700 Subject: [PATCH 14/29] Clean up the metadata queries that power is_iceberg column generation --- dbt/adapters/snowflake/impl.py | 36 ++++------------------- dbt/include/snowflake/macros/adapters.sql | 21 +++++++------ 2 files changed, 16 insertions(+), 41 deletions(-) diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index eb1443809..3abca8a3f 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -233,34 +233,8 @@ def list_relations_without_caching( ) -> List[SnowflakeRelation]: kwargs = {"schema_relation": schema_relation} - def check_is_iceberg(row, table2): - for match_row in table2.rows: - if ( - row["name"] == match_row["name"] - and row["database_name"] == match_row["database_name"] - and row["schema_name"] == match_row["schema_name"] - ): - return "Y" - return "N" - try: schema_objects = self.execute_macro(LIST_RELATIONS_MACRO_NAME, kwargs=kwargs) - iceberg_table_results = self.execute_macro( - LIST_ICEBERG_RELATIONS_MACRO_NAME, kwargs=kwargs - ) - import agate - - # this only seems to only inflate runtime 16%; TODO: stress test - results = schema_objects.compute( - [ - ( - "is_iceberg", - agate.Formula( - agate.Text(), lambda row: check_is_iceberg(row, iceberg_table_results) - ), - ) - ] - ) except DbtDatabaseError as exc: # if the schema doesn't exist, we just want to return. # Alternatively, we could query the list of schemas before we start @@ -271,12 +245,12 @@ def check_is_iceberg(row, table2): # this can be reduced to always including `is_dynamic` once bundle `2024_03` is mandatory columns = ["database_name", "schema_name", "name", "kind"] - if "is_dynamic" in results.column_names: + if "is_dynamic" in schema_objects.column_names: columns.append("is_dynamic") - if "is_iceberg" in results.column_names: + if "is_iceberg" in schema_objects.column_names: columns.append("is_iceberg") - return [self._parse_list_relations_result(result) for result in results.select(columns)] + return [self._parse_list_relations_result(obj) for obj in schema_objects.select(columns)] def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation: # this can be reduced to always including `is_dynamic` once bundle `2024_03` is mandatory @@ -296,7 +270,9 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation relation_type = self.Relation.DynamicTable table_format: str = ( - SnowflakeObjectFormat.ICEBERG if is_iceberg == "Y" else SnowflakeObjectFormat.DEFAULT + SnowflakeObjectFormat.ICEBERG + if is_iceberg in ("Y", "YES") + else SnowflakeObjectFormat.DEFAULT ) quote_policy = {"database": True, "schema": True, "identifier": True} diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index 1662b46a2..e52c4328c 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -139,11 +139,19 @@ {%- set max_total_results = max_results_per_iter * max_iter -%} {% if schema_relation is string %} {%- set sql -%} - show objects in {{ schema_relation }} limit {{ max_results_per_iter }} + show objects in {{ schema_relation }} limit {{ max_results_per_iter }}; + select all_objects.*, is_iceberg as "is_iceberg" + from table(result_scan(last_query_id(-1))) all_objects + left join INFORMATION_SCHEMA.tables as all_tables + on all_tables.TABLE_NAME = all_objects."name" {%- endset -%} {% else %} {%- set sql -%} - show objects in {{ schema_relation.include(identifier=False) }} limit {{ max_results_per_iter }} + show objects in {{ schema_relation.include(identifier=False) }} limit {{ max_results_per_iter }}; + select all_objects.*, is_iceberg as "is_iceberg" + from table(result_scan(last_query_id(-1))) all_objects + left join INFORMATION_SCHEMA.tables as all_tables + on all_tables.TABLE_NAME = all_objects."name" {%- endset -%} {% endif -%} @@ -318,12 +326,3 @@ {{ snowflake_dml_explicit_transaction(truncate_dml) }} {%- endcall %} {% endmacro %} - - -{% macro snowflake__show_iceberg_relations(schema_relation) %} - {%- set sql -%} - show iceberg tables in {{ schema_relation }} - {%- endset -%} - {%- set result = run_query(sql) -%} - {%- do return(result) -%} -{% endmacro %} From 4bf934ca9d7bfed42afd873c16c24ae2b6f23044 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 00:17:10 -0700 Subject: [PATCH 15/29] Fix Python models generation argument --- dbt/include/snowflake/macros/relations/table/create.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index f82a40277..24680efe8 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -54,10 +54,10 @@ {%- endif -%} {%- elif language == 'python' -%} - {%- if iceberg -%} + {%- if relation.is_iceberg_format %} {% do exceptions.raise_compiler_error('Iceberg is incompatible with Python models. Please use a SQL model for the iceberg format.') %} {%- endif %} - {{ py_write_table(compiled_code=compiled_code, target_relation=relation, table_type=get_create_ddl_prefix(temporary)) }} + {{ py_write_table(compiled_code=compiled_code, target_relation=relation, table_type=relation.get_ddl_prefix_for_create(config.model.config, temporary) }} {%- else -%} {% do exceptions.raise_compiler_error("snowflake__create_table_as macro didn't get supported language, it got %s" % language) %} {%- endif -%} From 493c6aeea26696dc18600ebbc80544f3f6217ab7 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 00:18:14 -0700 Subject: [PATCH 16/29] Add changelog. --- .changes/unreleased/Features-20240911-001806.yaml | 6 ++++++ dbt/include/snowflake/macros/relations/table/create.sql | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/Features-20240911-001806.yaml diff --git a/.changes/unreleased/Features-20240911-001806.yaml b/.changes/unreleased/Features-20240911-001806.yaml new file mode 100644 index 000000000..024480b96 --- /dev/null +++ b/.changes/unreleased/Features-20240911-001806.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Add support for Iceberg table materializations. +time: 2024-09-11T00:18:06.780586-07:00 +custom: + Author: versusfacit + Issue: "321" diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 24680efe8..8a7418c2b 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -57,7 +57,7 @@ {%- if relation.is_iceberg_format %} {% do exceptions.raise_compiler_error('Iceberg is incompatible with Python models. Please use a SQL model for the iceberg format.') %} {%- endif %} - {{ py_write_table(compiled_code=compiled_code, target_relation=relation, table_type=relation.get_ddl_prefix_for_create(config.model.config, temporary) }} + {{ py_write_table(compiled_code=compiled_code, target_relation=relation, table_type=relation.get_ddl_prefix_for_create(config.model.config, temporary)) }} {%- else -%} {% do exceptions.raise_compiler_error("snowflake__create_table_as macro didn't get supported language, it got %s" % language) %} {%- endif -%} From 66c2e5a59678714dc50c659be71bc5025bb548b4 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 00:56:09 -0700 Subject: [PATCH 17/29] Try to fix duplication of join record issues. --- dbt/include/snowflake/macros/adapters.sql | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index e52c4328c..a03fd2cb9 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -143,7 +143,9 @@ select all_objects.*, is_iceberg as "is_iceberg" from table(result_scan(last_query_id(-1))) all_objects left join INFORMATION_SCHEMA.tables as all_tables - on all_tables.TABLE_NAME = all_objects."name" + on all_tables.table_name = all_objects."name" + and all_tables.table_schema = all_objects."schema_name" + and all_tables.table_catalog = all_objects."database_name" {%- endset -%} {% else %} {%- set sql -%} @@ -151,7 +153,9 @@ select all_objects.*, is_iceberg as "is_iceberg" from table(result_scan(last_query_id(-1))) all_objects left join INFORMATION_SCHEMA.tables as all_tables - on all_tables.TABLE_NAME = all_objects."name" + on all_tables.table_name = all_objects."name" + and all_tables.table_schema = all_objects."schema_name" + and all_tables.table_catalog = all_objects."database_name" {%- endset -%} {% endif -%} From e913f28ffaaa38ddb8b9871ab344dd0cd971b9ad Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 01:13:54 -0700 Subject: [PATCH 18/29] Use the RelationConfig protocol for type checking. --- dbt/adapters/snowflake/relation.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 6a50fab88..169ba86ed 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -1,7 +1,7 @@ import textwrap from dataclasses import dataclass, field -from typing import FrozenSet, Optional, Type, TYPE_CHECKING +from typing import FrozenSet, Optional, Type from dbt.adapters.base.relation import BaseRelation @@ -27,9 +27,6 @@ SnowflakeRelationType, ) -if TYPE_CHECKING: - from dbt.artifacts.resources.v1.model import ModelConfig - @dataclass(frozen=True, eq=False, repr=False) class SnowflakeRelation(BaseRelation): @@ -135,7 +132,7 @@ def as_case_sensitive(self) -> "SnowflakeRelation": return self.replace_path(**path_part_map) - def get_ddl_prefix_for_create(self, config: "ModelConfig", temporary: bool): + def get_ddl_prefix_for_create(self, config: RelationConfig, temporary: bool): """ This macro renders the appropriate DDL prefix during the create_table_as macro. It decides based on mutually exclusive table configuration options: @@ -187,7 +184,7 @@ def get_ddl_prefix_for_alter(self): else: return "" - def render_iceberg_ddl(self, config: "ModelConfig"): + def render_iceberg_ddl(self, config: RelationConfig): base_location: str = f"_dbt/{self.schema}/{self.name}" if subpath := config.get("base_location_subpath"): From ebcc728ecd6e35da50242e67371c9d53f8a548e4 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 01:28:24 -0700 Subject: [PATCH 19/29] Fix transient semantics. --- dbt/adapters/snowflake/relation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 169ba86ed..6b310d542 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -172,7 +172,7 @@ def get_ddl_prefix_for_create(self, config: RelationConfig, temporary: bool): # Always supply transient on table create DDL unless user specifically sets # transient to false or unset. Might as well update the object attribute too! - elif transient_explicitly_set_true or config.get("transient") is None: + elif transient_explicitly_set_true or config.get("transient", True): return "transient" else: return "" From 1cf5b742fc1fcc93507d4f5e01e92cfdb4de8bbb Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 09:44:46 -0700 Subject: [PATCH 20/29] Add functional tests. --- tests/functional/iceberg/test_table_basic.py | 97 ++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 tests/functional/iceberg/test_table_basic.py diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py new file mode 100644 index 000000000..fe4431d06 --- /dev/null +++ b/tests/functional/iceberg/test_table_basic.py @@ -0,0 +1,97 @@ +import pytest + +from dbt.tests.util import run_dbt + + +_MODEL_BASIC_TABLE_MODEL = """ +{{ + config( + materialized = "table", + cluster_by=['id'], + ) +}} +select 1 as id +""".strip() + +_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') }} +""".strip() + +_MODEL_BUILT_ON_ICEBERG_TABLE = """ +{{ + config( + materialized = "table", + ) +}} +select * from {{ ref('iceberg_table') }} +""".strip() + +_MODEL_TABLE_FOR_SWAP = """ +{{ + config( + materialized = "table", + ) +}} +select 1 as id +""".strip() + +_MODEL_TABLE_FOR_SWAP_ICEBERG = """ +{{ + config( + materialized = "table", + table_format="iceberg", + external_volume="s3_iceberg_snow", + base_location_subpath="subpath", + ) +}} +select 1 as id +""".strip() + + +class TestIcebergTableBuilds: + @pytest.fixture(scope="class") + def models(self): + return { + "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, + } + + def test_iceberg_tables_build_and_can_be_referred(self, project): + run_results = run_dbt() + assert len(run_results) == 3 + + +class TestIcebergTableTypeBuildsOnExistingTable: + model_name = "my_model.sql" + + @pytest.fixture(scope="class") + def models(self): + return {self.model_name: _MODEL_TABLE_FOR_SWAP} + + def test_changing_model_types(self, project): + model_file = project.project_root / Path("models") / Path(self.model_name) + + run_results = run_dbt() + assert len(run_results) == 1 + + rm_file(model_file) + write_file(_MODEL_TABLE_FOR_SWAP_ICEBERG, model_file) + run_results = run_dbt() + assert len(run_results) == 1 + + rm_file(model_file) + write_file(_MODEL_TABLE_FOR_SWAP, model_file) + run_results = run_dbt() + assert len(run_results) == 1 From f198177ef58000c829c726ac2c525490f72573b2 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 10:19:33 -0700 Subject: [PATCH 21/29] Fix test. --- tests/functional/iceberg/test_table_basic.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py index fe4431d06..b5680ae95 100644 --- a/tests/functional/iceberg/test_table_basic.py +++ b/tests/functional/iceberg/test_table_basic.py @@ -1,5 +1,7 @@ import pytest +from pathlib import Path + from dbt.tests.util import run_dbt From 8a267546de7b6a7830a908bdadccd5f2ae52c64b Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 10:48:53 -0700 Subject: [PATCH 22/29] Fix test. --- tests/functional/iceberg/test_table_basic.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py index b5680ae95..0d291a6d5 100644 --- a/tests/functional/iceberg/test_table_basic.py +++ b/tests/functional/iceberg/test_table_basic.py @@ -2,8 +2,7 @@ from pathlib import Path -from dbt.tests.util import run_dbt - +from dbt.tests.util import run_dbt, rm_file _MODEL_BASIC_TABLE_MODEL = """ {{ From e4d98e5614858cd66c6b99c6de34a4e9b6de4bba Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 21:20:00 -0700 Subject: [PATCH 23/29] Fix test and remove strip calls --- tests/functional/iceberg/test_table_basic.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py index 0d291a6d5..5aad1a316 100644 --- a/tests/functional/iceberg/test_table_basic.py +++ b/tests/functional/iceberg/test_table_basic.py @@ -2,7 +2,7 @@ from pathlib import Path -from dbt.tests.util import run_dbt, rm_file +from dbt.tests.util import run_dbt, rm_file, write_file _MODEL_BASIC_TABLE_MODEL = """ {{ @@ -12,7 +12,7 @@ ) }} select 1 as id -""".strip() +""" _MODEL_BASIC_ICEBERG_MODEL = """ {{ @@ -27,7 +27,7 @@ }} select * from {{ ref('first_table') }} -""".strip() +""" _MODEL_BUILT_ON_ICEBERG_TABLE = """ {{ @@ -36,7 +36,7 @@ ) }} select * from {{ ref('iceberg_table') }} -""".strip() +""" _MODEL_TABLE_FOR_SWAP = """ {{ @@ -45,7 +45,7 @@ ) }} select 1 as id -""".strip() +""" _MODEL_TABLE_FOR_SWAP_ICEBERG = """ {{ @@ -57,7 +57,7 @@ ) }} select 1 as id -""".strip() +""" class TestIcebergTableBuilds: From 92d7bc055a2ca13ddc276a2083eb4ba1889d8ac8 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 21:26:57 -0700 Subject: [PATCH 24/29] Add view test case. --- tests/functional/iceberg/test_table_basic.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py index 5aad1a316..c02907c3b 100644 --- a/tests/functional/iceberg/test_table_basic.py +++ b/tests/functional/iceberg/test_table_basic.py @@ -38,7 +38,7 @@ select * from {{ ref('iceberg_table') }} """ -_MODEL_TABLE_FOR_SWAP = """ +_MODEL_TABLE_BEFORE_SWAP = """ {{ config( materialized = "table", @@ -47,6 +47,10 @@ select 1 as id """ +_MODEL_VIEW_BEFORE_SWAP = """ +select 1 as id +""" + _MODEL_TABLE_FOR_SWAP_ICEBERG = """ {{ config( @@ -77,13 +81,11 @@ def test_iceberg_tables_build_and_can_be_referred(self, project): class TestIcebergTableTypeBuildsOnExistingTable: model_name = "my_model.sql" - @pytest.fixture(scope="class") - def models(self): - return {self.model_name: _MODEL_TABLE_FOR_SWAP} - - def test_changing_model_types(self, project): + @pytest.mark.parametrize("start_model", [_MODEL_TABLE_BEFORE_SWAP, _MODEL_VIEW_BEFORE_SWAP]) + def test_changing_model_types(self, project, start_model): model_file = project.project_root / Path("models") / Path(self.model_name) + write_file(start_model, model_file) run_results = run_dbt() assert len(run_results) == 1 @@ -93,6 +95,6 @@ def test_changing_model_types(self, project): assert len(run_results) == 1 rm_file(model_file) - write_file(_MODEL_TABLE_FOR_SWAP, model_file) + write_file(start_model, model_file) run_results = run_dbt() assert len(run_results) == 1 From e961bb0be261e3876f40683f3b9dd5701e37f164 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 22:46:52 -0700 Subject: [PATCH 25/29] Code review comments. --- dbt/adapters/snowflake/impl.py | 7 +- dbt/adapters/snowflake/relation.py | 72 ++++++++++++++++--- .../snowflake/relation_configs/__init__.py | 2 +- .../snowflake/relation_configs/formats.py | 7 +- dbt/include/snowflake/macros/adapters.sql | 18 ++--- .../macros/materializations/table.sql | 35 +-------- .../macros/relations/table/create.sql | 3 +- .../list_relations_tests/test_show_objects.py | 2 +- 8 files changed, 82 insertions(+), 64 deletions(-) diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index 3abca8a3f..9b9a27682 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -22,7 +22,7 @@ from dbt.adapters.snowflake.relation_configs import ( SnowflakeRelationType, - SnowflakeObjectFormat, + TableFormat, ) from dbt.adapters.snowflake import SnowflakeColumn from dbt.adapters.snowflake import SnowflakeConnectionManager @@ -32,7 +32,6 @@ import agate SHOW_OBJECT_METADATA_MACRO_NAME = "snowflake__show_object_metadata" -LIST_ICEBERG_RELATIONS_MACRO_NAME = "snowflake__show_iceberg_relations" @dataclass @@ -270,9 +269,7 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation relation_type = self.Relation.DynamicTable table_format: str = ( - SnowflakeObjectFormat.ICEBERG - if is_iceberg in ("Y", "YES") - else SnowflakeObjectFormat.DEFAULT + TableFormat.ICEBERG if is_iceberg in ("Y", "YES") else TableFormat.DEFAULT ) quote_policy = {"database": True, "schema": True, "identifier": True} diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 6b310d542..98601cf5b 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -1,12 +1,12 @@ import textwrap from dataclasses import dataclass, field -from typing import FrozenSet, Optional, Type +from typing import FrozenSet, Optional, Type, Self, Iterator from dbt.adapters.base.relation import BaseRelation from dbt.adapters.contracts.relation import ComponentName, RelationConfig -from dbt.adapters.events.types import AdapterEventWarning +from dbt.adapters.events.types import AdapterEventWarning, AdapterEventDebug from dbt.adapters.relation_configs import ( RelationConfigBase, RelationConfigChangeAction, @@ -14,7 +14,7 @@ ) from dbt.adapters.utils import classproperty from dbt_common.exceptions import DbtRuntimeError -from dbt_common.events.functions import warn_or_error +from dbt_common.events.functions import fire_event, warn_or_error from dbt.adapters.snowflake.relation_configs import ( SnowflakeDynamicTableConfig, @@ -22,7 +22,7 @@ SnowflakeDynamicTableRefreshModeConfigChange, SnowflakeDynamicTableTargetLagConfigChange, SnowflakeDynamicTableWarehouseConfigChange, - SnowflakeObjectFormat, + TableFormat, SnowflakeQuotePolicy, SnowflakeRelationType, ) @@ -31,7 +31,7 @@ @dataclass(frozen=True, eq=False, repr=False) class SnowflakeRelation(BaseRelation): type: Optional[SnowflakeRelationType] = None - table_format: str = SnowflakeObjectFormat.DEFAULT + table_format: str = TableFormat.DEFAULT quote_policy: SnowflakeQuotePolicy = field(default_factory=lambda: SnowflakeQuotePolicy()) require_alias: bool = False relation_configs = { @@ -62,7 +62,7 @@ def is_dynamic_table(self) -> bool: @property def is_iceberg_format(self) -> bool: - return self.table_format == SnowflakeObjectFormat.ICEBERG + return self.table_format == TableFormat.ICEBERG @classproperty def DynamicTable(cls) -> str: @@ -132,7 +132,7 @@ def as_case_sensitive(self) -> "SnowflakeRelation": return self.replace_path(**path_part_map) - def get_ddl_prefix_for_create(self, config: RelationConfig, temporary: bool): + def get_ddl_prefix_for_create(self, config: RelationConfig, temporary: bool) -> str: """ This macro renders the appropriate DDL prefix during the create_table_as macro. It decides based on mutually exclusive table configuration options: @@ -177,14 +177,14 @@ def get_ddl_prefix_for_create(self, config: RelationConfig, temporary: bool): else: return "" - def get_ddl_prefix_for_alter(self): + def get_ddl_prefix_for_alter(self) -> str: """All ALTER statements on Iceberg tables require an ICEBERG prefix""" if self.is_iceberg_format: return "iceberg" else: return "" - def render_iceberg_ddl(self, config: RelationConfig): + def get_iceberg_ddl_options(self, config: RelationConfig) -> str: base_location: str = f"_dbt/{self.schema}/{self.name}" if subpath := config.get("base_location_subpath"): @@ -196,3 +196,57 @@ def render_iceberg_ddl(self, config: RelationConfig): base_location = '{base_location}' """ return textwrap.indent(textwrap.dedent(iceberg_ddl_predicates), " " * 10) + + def __drop_conditions(self, old_relation: Self) -> Iterator[tuple[bool, str]]: + drop_view_message: str = ( + f"Dropping relation {old_relation} because it is a view and target relation {self} " + f"is of type {self.type}." + ) + + drop_table_for_iceberg_message: str = ( + f"Dropping relation {old_relation} because it is a default format table " + f"and target relation {self} is an Iceberg format table." + ) + + drop_iceberg_for_table_message: str = ( + f"Dropping relation {old_relation} because it is an Iceberg format table " + f"and target relation {self} is a default format table." + ) + + # An existing view must be dropped for model to build into a table". + yield (not old_relation.is_table, drop_view_message) + # An existing table must be dropped for model to build into an Iceberg table. + yield ( + old_relation.is_table + and not old_relation.is_iceberg_format + and self.is_iceberg_format, + drop_table_for_iceberg_message, + ) + # existing Iceberg table must be dropped for model to build into a table. + yield ( + old_relation.is_table + and old_relation.is_iceberg_format + and not self.is_iceberg_format, + drop_iceberg_for_table_message, + ) + + def needs_to_drop(self, old_relation: Optional[Self]) -> bool: + """ + To convert between Iceberg and non-Iceberg relations, a preemptive drop is + required. + + drops cause latency, but it should be a relatively infrequent occurrence. + + Some Boolean expression below are logically redundant, but this is done for easier + readability. + """ + + if old_relation is None: + return False + + for condition, message in self.__drop_conditions(old_relation): + if condition: + fire_event(AdapterEventDebug(base_msg=message)) + return True + + return False diff --git a/dbt/adapters/snowflake/relation_configs/__init__.py b/dbt/adapters/snowflake/relation_configs/__init__.py index f2b3c85dc..61941ab50 100644 --- a/dbt/adapters/snowflake/relation_configs/__init__.py +++ b/dbt/adapters/snowflake/relation_configs/__init__.py @@ -10,4 +10,4 @@ SnowflakeQuotePolicy, SnowflakeRelationType, ) -from dbt.adapters.snowflake.relation_configs.formats import SnowflakeObjectFormat +from dbt.adapters.snowflake.relation_configs.formats import TableFormat diff --git a/dbt/adapters/snowflake/relation_configs/formats.py b/dbt/adapters/snowflake/relation_configs/formats.py index 05ed50c5d..460241d9d 100644 --- a/dbt/adapters/snowflake/relation_configs/formats.py +++ b/dbt/adapters/snowflake/relation_configs/formats.py @@ -1,7 +1,12 @@ from dbt_common.dataclass_schema import StrEnum # doesn't exist in standard library until py3.11 -class SnowflakeObjectFormat(StrEnum): +class TableFormat(StrEnum): + """ + Snowflake docs refers to this an 'Object Format.' + Data practitioners and interfaces refer to this as 'Table Format's, hence the term's use here. + """ + DEFAULT = "default" ICEBERG = "iceberg" diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index a03fd2cb9..930df8d5d 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -137,27 +137,19 @@ {% macro snowflake__list_relations_without_caching(schema_relation, max_iter=10, max_results_per_iter=10000) %} {%- set max_total_results = max_results_per_iter * max_iter -%} - {% if schema_relation is string %} - {%- set sql -%} + {%- set sql -%} + {% if schema_relation is string %} show objects in {{ schema_relation }} limit {{ max_results_per_iter }}; - select all_objects.*, is_iceberg as "is_iceberg" - from table(result_scan(last_query_id(-1))) all_objects - left join INFORMATION_SCHEMA.tables as all_tables - on all_tables.table_name = all_objects."name" - and all_tables.table_schema = all_objects."schema_name" - and all_tables.table_catalog = all_objects."database_name" - {%- endset -%} - {% else %} - {%- set sql -%} + {% else %} show objects in {{ schema_relation.include(identifier=False) }} limit {{ max_results_per_iter }}; + {% endif -%} select all_objects.*, is_iceberg as "is_iceberg" from table(result_scan(last_query_id(-1))) all_objects left join INFORMATION_SCHEMA.tables as all_tables on all_tables.table_name = all_objects."name" and all_tables.table_schema = all_objects."schema_name" and all_tables.table_catalog = all_objects."database_name" - {%- endset -%} - {% endif -%} + {%- endset -%} {%- set result = run_query(sql) -%} diff --git a/dbt/include/snowflake/macros/materializations/table.sql b/dbt/include/snowflake/macros/materializations/table.sql index 9cadeff0f..cbc6d9ce6 100644 --- a/dbt/include/snowflake/macros/materializations/table.sql +++ b/dbt/include/snowflake/macros/materializations/table.sql @@ -18,7 +18,9 @@ {{ run_hooks(pre_hooks) }} - {{ drop_old_relation_as_needed(old_relation, target_relation) }} + {% if target_relation.needs_to_drop(old_relation) %} + {{ drop_relation_if_exists(old_relation) }} + {% endif %} {% call statement('main', language=language) -%} {{ create_table_as(False, target_relation, compiled_code, language) }} @@ -84,34 +86,3 @@ def main(session): # dbt = dbtObj(session.table) # df = model(dbt, session) {%endmacro%} - - -{% macro drop_old_relation_as_needed(old_relation, target_relation) %} - {% if old_relation is none %} - {{ return('') }} - {% endif %} - - {# - -- Each of these will cause some latency, but it shoudl be a relatively infrequent occurrence. - - -- An existing view must be dropped for model to "convert" into a table" - #} - {% if not old_relation.is_table %} - {{ log("Dropping relation " ~ old_relation ~ " because it is of type " ~ old_relation.type) }} - {{ drop_relation_if_exists(old_relation) }} - - {# - -- An existing Iceberg table must be dropped for model to "convert" into a table. - #} - {% elif old_relation.is_iceberg_format and not target_relation.is_iceberg_format %} - {{ log("Dropping relation " ~ old_relation ~ " because it is an Iceberg format table and target relation " ~ target_relation ~ " is a default format table.") }} - {{ drop_relation_if_exists(old_relation) }} - - {# - -- An existing table must be dropped for model to "convert" into an Iceberg table. - #} - {% elif old_relation.is_table and not old_relation.is_iceberg_format and target_relation.is_iceberg_format %} - {{ log("Dropping relation " ~ old_relation ~ " because it is a default format table and target relation is an Iceberg format table.") }} - {{ drop_relation_if_exists(old_relation) }} - {% endif %} -{% endmacro %} diff --git a/dbt/include/snowflake/macros/relations/table/create.sql b/dbt/include/snowflake/macros/relations/table/create.sql index 8a7418c2b..355150e28 100644 --- a/dbt/include/snowflake/macros/relations/table/create.sql +++ b/dbt/include/snowflake/macros/relations/table/create.sql @@ -26,8 +26,7 @@ Valid DDL in CTAS statements. Plain create statements have a different order. https://docs.snowflake.com/en/sql-reference/sql/create-iceberg-table #} - {{ relation.render_iceberg_ddl(config.model.config) }} - {% else %} + {{ relation.get_iceberg_ddl_options(config.model.config) }} {%- endif -%} {%- set contract_config = config.get('contract') -%} diff --git a/tests/functional/adapter/list_relations_tests/test_show_objects.py b/tests/functional/adapter/list_relations_tests/test_show_objects.py index f59dc335b..e5eee39d9 100644 --- a/tests/functional/adapter/list_relations_tests/test_show_objects.py +++ b/tests/functional/adapter/list_relations_tests/test_show_objects.py @@ -73,7 +73,7 @@ def list_relations_without_caching(project) -> List[SnowflakeRelation]: database=project.database, schema=project.test_schema, identifier="" ) with get_connection(my_adapter): - relations = my_adapter.list_relations_without_caching(schema.path.schema) + relations = my_adapter.list_relations_without_caching(schema) return relations def test_list_relations_without_caching(self, project): From 4a6046f433b97aeae42e26d3b06390c117e488a2 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 22:50:52 -0700 Subject: [PATCH 26/29] I'm using too new a version of mypy for Self. --- dbt/adapters/snowflake/relation.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dbt/adapters/snowflake/relation.py b/dbt/adapters/snowflake/relation.py index 98601cf5b..224b2b75e 100644 --- a/dbt/adapters/snowflake/relation.py +++ b/dbt/adapters/snowflake/relation.py @@ -1,7 +1,7 @@ import textwrap from dataclasses import dataclass, field -from typing import FrozenSet, Optional, Type, Self, Iterator +from typing import FrozenSet, Optional, Type, Iterator, Tuple from dbt.adapters.base.relation import BaseRelation @@ -197,7 +197,7 @@ def get_iceberg_ddl_options(self, config: RelationConfig) -> str: """ return textwrap.indent(textwrap.dedent(iceberg_ddl_predicates), " " * 10) - def __drop_conditions(self, old_relation: Self) -> Iterator[tuple[bool, str]]: + def __drop_conditions(self, old_relation: "SnowflakeRelation") -> Iterator[Tuple[bool, str]]: drop_view_message: str = ( f"Dropping relation {old_relation} because it is a view and target relation {self} " f"is of type {self.type}." @@ -230,7 +230,7 @@ def __drop_conditions(self, old_relation: Self) -> Iterator[tuple[bool, str]]: drop_iceberg_for_table_message, ) - def needs_to_drop(self, old_relation: Optional[Self]) -> bool: + def needs_to_drop(self, old_relation: Optional["SnowflakeRelation"]) -> bool: """ To convert between Iceberg and non-Iceberg relations, a preemptive drop is required. From d0c39f3ec5c7f516803624e1538f7739869ffe36 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Wed, 11 Sep 2024 23:44:02 -0700 Subject: [PATCH 27/29] Add a behavior flag for iceberg table materialization. --- dbt/adapters/snowflake/impl.py | 20 +++++++++++++++++--- dbt/include/snowflake/macros/adapters.sql | 5 +++++ tests/functional/iceberg/test_table_basic.py | 9 ++++++++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index 9b9a27682..42a46d553 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -9,6 +9,7 @@ LIST_SCHEMAS_MACRO_NAME, LIST_RELATIONS_MACRO_NAME, ) +from dbt_common.behavior_flags import BehaviorFlag from dbt_common.contracts.constraints import ConstraintType from dbt_common.contracts.metadata import ( TableMetadata, @@ -77,6 +78,10 @@ class SnowflakeAdapter(SQLAdapter): } ) + @property + def _behavior_flags(self) -> List[BehaviorFlag]: + return [{"name": "enable_iceberg_materializations", "default": False}] + @classmethod def date_function(cls): return "CURRENT_TIMESTAMP()" @@ -253,12 +258,17 @@ def list_relations_without_caching( def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation: # this can be reduced to always including `is_dynamic` once bundle `2024_03` is mandatory + # this can be reduced to always including `is_iceberg` once Snowflake adds it to show objects try: - database, schema, identifier, relation_type, is_dynamic, is_iceberg = result + if self.behavior.enable_iceberg_materializations.no_warn: + database, schema, identifier, relation_type, is_dynamic = result + else: + database, schema, identifier, relation_type, is_dynamic, is_iceberg = result except ValueError: database, schema, identifier, relation_type = result is_dynamic = "N" - is_iceberg = "N" + if self.behavior.enable_iceberg_materializations.no_warn: + is_iceberg = "N" try: relation_type = self.Relation.get_relation_type(relation_type.lower()) @@ -268,8 +278,12 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation if relation_type == self.Relation.Table and is_dynamic == "Y": relation_type = self.Relation.DynamicTable + # This line is the main gate on supporting Iceberg materializations. Pass forward a default + # table format, and no downstream table macros can build iceberg relations. table_format: str = ( - TableFormat.ICEBERG if is_iceberg in ("Y", "YES") else TableFormat.DEFAULT + TableFormat.ICEBERG + if self.behavior.enable_iceberg_materializations.no_warn and is_iceberg in ("Y", "YES") + else TableFormat.DEFAULT ) quote_policy = {"database": True, "schema": True, "identifier": True} diff --git a/dbt/include/snowflake/macros/adapters.sql b/dbt/include/snowflake/macros/adapters.sql index 930df8d5d..aa8895819 100644 --- a/dbt/include/snowflake/macros/adapters.sql +++ b/dbt/include/snowflake/macros/adapters.sql @@ -143,12 +143,17 @@ {% else %} show objects in {{ schema_relation.include(identifier=False) }} limit {{ max_results_per_iter }}; {% endif -%} + + {# -- Gated for performance reason. If you don't want Iceberg, you shouldn't pay the + -- latency penalty. #} + {% if adapter.behavior.enable_iceberg_materializations.no_warn %} select all_objects.*, is_iceberg as "is_iceberg" from table(result_scan(last_query_id(-1))) all_objects left join INFORMATION_SCHEMA.tables as all_tables on all_tables.table_name = all_objects."name" and all_tables.table_schema = all_objects."schema_name" and all_tables.table_catalog = all_objects."database_name" + {% endif -%} {%- endset -%} {%- set result = run_query(sql) -%} diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py index c02907c3b..a37b5b2b3 100644 --- a/tests/functional/iceberg/test_table_basic.py +++ b/tests/functional/iceberg/test_table_basic.py @@ -65,6 +65,10 @@ class TestIcebergTableBuilds: + @pytest.fixture(scope="class") + def project_config_update(self): + return {"flags": {"enable_iceberg_materializations": True}} + @pytest.fixture(scope="class") def models(self): return { @@ -79,10 +83,13 @@ def test_iceberg_tables_build_and_can_be_referred(self, project): class TestIcebergTableTypeBuildsOnExistingTable: - model_name = "my_model.sql" + @pytest.fixture(scope="class") + def project_config_update(self): + return {"flags": {"enable_iceberg_materializations": True}} @pytest.mark.parametrize("start_model", [_MODEL_TABLE_BEFORE_SWAP, _MODEL_VIEW_BEFORE_SWAP]) def test_changing_model_types(self, project, start_model): + model_name = "my_model.sql" model_file = project.project_root / Path("models") / Path(self.model_name) write_file(start_model, model_file) From 17cd094b2499cdb0f8a8aa8eb14fcd8532f91ac1 Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Thu, 12 Sep 2024 00:12:54 -0700 Subject: [PATCH 28/29] Flip order of flag. --- dbt/adapters/snowflake/impl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/adapters/snowflake/impl.py b/dbt/adapters/snowflake/impl.py index 42a46d553..7e8ec9cf2 100644 --- a/dbt/adapters/snowflake/impl.py +++ b/dbt/adapters/snowflake/impl.py @@ -261,9 +261,9 @@ def _parse_list_relations_result(self, result: "agate.Row") -> SnowflakeRelation # this can be reduced to always including `is_iceberg` once Snowflake adds it to show objects try: if self.behavior.enable_iceberg_materializations.no_warn: - database, schema, identifier, relation_type, is_dynamic = result - else: database, schema, identifier, relation_type, is_dynamic, is_iceberg = result + else: + database, schema, identifier, relation_type, is_dynamic = result except ValueError: database, schema, identifier, relation_type = result is_dynamic = "N" From a623bb5d8262bf55fdd284cde95ad0756091386c Mon Sep 17 00:00:00 2001 From: VersusFacit Date: Thu, 12 Sep 2024 11:57:25 -0700 Subject: [PATCH 29/29] Adjust test. --- tests/functional/iceberg/test_table_basic.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/functional/iceberg/test_table_basic.py b/tests/functional/iceberg/test_table_basic.py index a37b5b2b3..0bfdf59f1 100644 --- a/tests/functional/iceberg/test_table_basic.py +++ b/tests/functional/iceberg/test_table_basic.py @@ -89,8 +89,7 @@ def project_config_update(self): @pytest.mark.parametrize("start_model", [_MODEL_TABLE_BEFORE_SWAP, _MODEL_VIEW_BEFORE_SWAP]) def test_changing_model_types(self, project, start_model): - model_name = "my_model.sql" - model_file = project.project_root / Path("models") / Path(self.model_name) + model_file = project.project_root / Path("models") / Path("my_model.sql") write_file(start_model, model_file) run_results = run_dbt()