Skip to content

Commit 3785d4c

Browse files
committed
fix(snowflake): use ALTER ICEBERG TABLE to register column comments on Iceberg tables
Snowflake rejects `ALTER TABLE` for Iceberg tables and requires `ALTER ICEBERG TABLE` instead. `SnowflakeEngineAdapter._create_column_comments` builds its statement with a hardcoded `TABLE` kind, so registering column comments on an Iceberg table with a post-creation command emitted `ALTER TABLE ... ALTER COLUMN ... COMMENT`. The failure is swallowed and logged as a permissions warning, so the comments were silently dropped. This path is only taken when the comments can't be inlined into the CTAS schema definition, i.e. when the column types are not fully known. `_create_column_comments` now accepts `table_format`, mirroring the `_create_table` convention, and the base adapter passes it from the two table creation paths. The Snowflake adapter derives the Iceberg-specific table kind from it. Follow-up to #5722. Related: #5721 Signed-off-by: Guillem G <guillem.gimenez@titanos.tv>
1 parent c0ae2db commit 3785d4c

5 files changed

Lines changed: 58 additions & 2 deletions

File tree

‎sqlmesh/core/engine_adapter/base.py‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,9 @@ def _create_table_from_columns(
833833
and self.COMMENT_CREATION_TABLE.is_comment_command_only
834834
and self.comments_enabled
835835
):
836-
self._create_column_comments(table_name, column_descriptions)
836+
self._create_column_comments(
837+
table_name, column_descriptions, table_format=kwargs.get("table_format")
838+
)
837839

838840
def _build_schema_exp(
839841
self,
@@ -990,7 +992,9 @@ def _create_table_from_source_queries(
990992
):
991993
self._create_table_comment(table_name, table_description)
992994
if column_descriptions and schema is None and self.comments_enabled:
993-
self._create_column_comments(table_name, column_descriptions)
995+
self._create_column_comments(
996+
table_name, column_descriptions, table_format=kwargs.get("table_format")
997+
)
994998

995999
def _create_table(
9961000
self,
@@ -3058,7 +3062,18 @@ def _create_column_comments(
30583062
column_comments: t.Dict[str, str],
30593063
table_kind: str = "TABLE",
30603064
materialized_view: bool = False,
3065+
table_format: t.Optional[str] = None,
30613066
) -> None:
3067+
"""Registers column comments with a post-creation command.
3068+
3069+
Args:
3070+
table_name: The name of the table or view.
3071+
column_comments: Mapping between the column name and its comment.
3072+
table_kind: The kind of object being commented on, `TABLE` or `VIEW`.
3073+
materialized_view: Whether the view is materialized.
3074+
table_format: The table format of the table, if any. Engines that require
3075+
format-specific DDL to alter a table use it to derive `table_kind`.
3076+
"""
30623077
table = exp.to_table(table_name)
30633078

30643079
for col, comment in column_comments.items():

‎sqlmesh/core/engine_adapter/bigquery.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,7 @@ def _create_column_comments(
818818
column_comments: t.Dict[str, str],
819819
table_kind: str = "TABLE",
820820
materialized_view: bool = False,
821+
table_format: t.Optional[str] = None,
821822
) -> None:
822823
if not (table_kind == "VIEW" and materialized_view):
823824
table = self._get_table(table_name)

‎sqlmesh/core/engine_adapter/mysql.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ def _create_column_comments(
131131
column_comments: t.Dict[str, str],
132132
table_kind: str = "TABLE",
133133
materialized_view: bool = False,
134+
table_format: t.Optional[str] = None,
134135
) -> None:
135136
table = exp.to_table(table_name)
136137
table_sql = table.sql(dialect=self.dialect, identify=True)

‎sqlmesh/core/engine_adapter/snowflake.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,13 +632,20 @@ def _create_column_comments(
632632
column_comments: t.Dict[str, str],
633633
table_kind: str = "TABLE",
634634
materialized_view: bool = False,
635+
table_format: t.Optional[str] = None,
635636
) -> None:
636637
"""
637638
Reference: https://docs.snowflake.com/en/sql-reference/sql/alter-table-column#syntax
639+
Reference: https://docs.snowflake.com/en/sql-reference/sql/alter-iceberg-table#syntax
638640
"""
639641
if not column_comments:
640642
return
641643

644+
# Snowflake rejects `ALTER TABLE` for Iceberg tables, it requires
645+
# `ALTER ICEBERG TABLE` instead
646+
if table_format and table_kind == "TABLE":
647+
table_kind = f"{table_format.upper()} TABLE"
648+
642649
table = exp.to_table(table_name)
643650
table_sql = self._to_sql(table)
644651

‎tests/core/engine_adapter/test_snowflake.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,38 @@ def test_multiple_column_comments(make_mocked_engine_adapter: t.Callable, mocker
246246
]
247247

248248

249+
def test_column_comments_iceberg(make_mocked_engine_adapter: t.Callable):
250+
adapter = make_mocked_engine_adapter(SnowflakeEngineAdapter)
251+
252+
adapter._create_column_comments(
253+
"test_table",
254+
{"a": "a column description", "b": "b column description"},
255+
table_format="iceberg",
256+
)
257+
258+
assert to_sql_calls(adapter) == [
259+
"""ALTER ICEBERG TABLE "test_table" ALTER COLUMN "a" COMMENT 'a column description', COLUMN "b" COMMENT 'b column description'""",
260+
]
261+
262+
263+
def test_ctas_column_comments_iceberg(make_mocked_engine_adapter: t.Callable):
264+
adapter = make_mocked_engine_adapter(SnowflakeEngineAdapter)
265+
266+
# The column types are unknown, so the comments can't be inlined into the CTAS
267+
# schema definition and are registered with a post-creation ALTER instead
268+
adapter.ctas(
269+
"test_table",
270+
parse_one("SELECT a, b FROM source_table"),
271+
table_format="iceberg",
272+
column_descriptions={"a": "a column description"},
273+
)
274+
275+
assert to_sql_calls(adapter) == [
276+
"""CREATE ICEBERG TABLE IF NOT EXISTS "test_table" AS SELECT "a", "b" FROM "source_table\"""",
277+
"""ALTER ICEBERG TABLE "test_table" ALTER COLUMN "a" COMMENT 'a column description'""",
278+
]
279+
280+
249281
def test_sync_grants_config(make_mocked_engine_adapter: t.Callable, mocker: MockerFixture):
250282
adapter = make_mocked_engine_adapter(SnowflakeEngineAdapter)
251283
relation = normalize_identifiers(

0 commit comments

Comments
 (0)