Skip to content

Commit 52bfbfa

Browse files
sinelineStuffbyYuki
authored andcommitted
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 8c71de8 commit 52bfbfa

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,
@@ -3083,7 +3087,18 @@ def _create_column_comments(
30833087
column_comments: t.Dict[str, str],
30843088
table_kind: str = "TABLE",
30853089
materialized_view: bool = False,
3090+
table_format: t.Optional[str] = None,
30863091
) -> None:
3092+
"""Registers column comments with a post-creation command.
3093+
3094+
Args:
3095+
table_name: The name of the table or view.
3096+
column_comments: Mapping between the column name and its comment.
3097+
table_kind: The kind of object being commented on, `TABLE` or `VIEW`.
3098+
materialized_view: Whether the view is materialized.
3099+
table_format: The table format of the table, if any. Engines that require
3100+
format-specific DDL to alter a table use it to derive `table_kind`.
3101+
"""
30873102
table = exp.to_table(table_name)
30883103

30893104
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
@@ -819,6 +819,7 @@ def _create_column_comments(
819819
column_comments: t.Dict[str, str],
820820
table_kind: str = "TABLE",
821821
materialized_view: bool = False,
822+
table_format: t.Optional[str] = None,
822823
) -> None:
823824
if not (table_kind == "VIEW" and materialized_view):
824825
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
@@ -637,13 +637,20 @@ def _create_column_comments(
637637
column_comments: t.Dict[str, str],
638638
table_kind: str = "TABLE",
639639
materialized_view: bool = False,
640+
table_format: t.Optional[str] = None,
640641
) -> None:
641642
"""
642643
Reference: https://docs.snowflake.com/en/sql-reference/sql/alter-table-column#syntax
644+
Reference: https://docs.snowflake.com/en/sql-reference/sql/alter-iceberg-table#syntax
643645
"""
644646
if not column_comments:
645647
return
646648

649+
# Snowflake rejects `ALTER TABLE` for Iceberg tables, it requires
650+
# `ALTER ICEBERG TABLE` instead
651+
if table_format and table_kind == "TABLE":
652+
table_kind = f"{table_format.upper()} TABLE"
653+
647654
table = exp.to_table(table_name)
648655
table_sql = self._to_sql(table)
649656

‎tests/core/engine_adapter/test_snowflake.py‎

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

311311

312+
def test_column_comments_iceberg(make_mocked_engine_adapter: t.Callable):
313+
adapter = make_mocked_engine_adapter(SnowflakeEngineAdapter)
314+
315+
adapter._create_column_comments(
316+
"test_table",
317+
{"a": "a column description", "b": "b column description"},
318+
table_format="iceberg",
319+
)
320+
321+
assert to_sql_calls(adapter) == [
322+
"""ALTER ICEBERG TABLE "test_table" ALTER COLUMN "a" COMMENT 'a column description', COLUMN "b" COMMENT 'b column description'""",
323+
]
324+
325+
326+
def test_ctas_column_comments_iceberg(make_mocked_engine_adapter: t.Callable):
327+
adapter = make_mocked_engine_adapter(SnowflakeEngineAdapter)
328+
329+
# The column types are unknown, so the comments can't be inlined into the CTAS
330+
# schema definition and are registered with a post-creation ALTER instead
331+
adapter.ctas(
332+
"test_table",
333+
parse_one("SELECT a, b FROM source_table"),
334+
table_format="iceberg",
335+
column_descriptions={"a": "a column description"},
336+
)
337+
338+
assert to_sql_calls(adapter) == [
339+
"""CREATE ICEBERG TABLE IF NOT EXISTS "test_table" AS SELECT "a", "b" FROM "source_table\"""",
340+
"""ALTER ICEBERG TABLE "test_table" ALTER COLUMN "a" COMMENT 'a column description'""",
341+
]
342+
343+
312344
def test_sync_grants_config(make_mocked_engine_adapter: t.Callable, mocker: MockerFixture):
313345
adapter = make_mocked_engine_adapter(SnowflakeEngineAdapter)
314346
relation = normalize_identifiers(

0 commit comments

Comments
 (0)