Fix/snowflake iceberg clone dialect error

[sineline]

This pull request introduces enhancements to support Iceberg table operations in Snowflake, ensuring correct DDL syntax and behavior for table creation, cloning, and schema alterations. The changes primarily address the unique requirements of Iceberg tables, such as proper handling of PARTITION BY clauses and the use of ICEBERG TABLE syntax in relevant commands. Additionally, the pull request propagates table format information throughout the snapshot evaluation and migration processes.

Snowflake Iceberg Table Support:

• Added custom DDL logic in _create_table to correctly inject PARTITION BY clauses for Iceberg tables, ensuring compliance with Snowflake's ordering requirements and handling CTAS limitations.
• Updated clone_table to use CREATE ICEBERG TABLE ... CLONE syntax when cloning Iceberg tables, by passing table_kind based on the table format.
• Implemented a Snowflake-specific alter_table method that uses ALTER ICEBERG TABLE instead of ALTER TABLE for schema changes on Iceberg tables.

Propagation of Table Format:

• Modified snapshot evaluator logic to pass table_format during snapshot cloning and table migration, enabling downstream methods to select the correct DDL syntax. [1] [2]

Base Adapter Improvements:

• Updated clone_table and alter_table signatures in the base engine adapter to accept and propagate table_kind and table_format parameters, allowing engine-specific logic to be triggered as needed. [1] [2]

[CLAassistant]

All committers have signed the CLA.

[izeigerman]

any reason to make this change?

[sineline]

No good reason — that was unrelated noise that crept in. Removed.

[izeigerman]

why do we need to ignore this?

[sineline]

It was covering up the signature mismatch you spotted below, so it's gone now that the base and the override agree. There's no type: ignore left anywhere in the diff.

[izeigerman]

as far as I can tell this method in the base class doesn't accept kwargs. Shouldn't we change this?

[sineline]

Agreed, and this was the root of it. I've changed the base class rather than the override — EngineAdapter.alter_table now takes table_format: t.Optional[str] = None, and clone_table takes table_format/table_kind, mirroring the table_kind convention _create_table already uses. The Snowflake overrides now have identical signatures to their supertypes, so the type: ignore above could go.

I went with a named typed parameter rather than **kwargs — happy to switch if you'd prefer the latter. Either way the sibling overrides had to widen too (bigquery, clickhouse, fabric for alter_table; databricks for clone_table), otherwise mypy rejects them as incompatible with the supertype. That's what the earlier version of this PR got wrong, and why the type: ignore was there in the first place.

[StuffbyYuki]

@sineline It looks like @izeigerman provided feedback. Could you take a look?

[sineline]

@StuffbyYuki I've been procrastinating too much on this. I'll see if I can pick it next week. Thanks for the nudge!

[sineline]

@izeigerman @StuffbyYuki sorry for the long delay. I've rebased this onto current main and rewritten it as a single commit that addresses all three review comments.

1. "any reason to make this change?" (the query_factory lambda reformat in base.py)

No — that was unrelated noise. Dropped entirely.

2. "why do we need to ignore this?" (# type: ignore[override]) and 3. "the base class doesn't accept kwargs. Shouldn't we change this?"

You were right on both, and they were really the same problem: the override didn't match the base signature, and the type: ignore was papering over it. Rather than threading **kwargs through, I changed the base signatures explicitly, following the existing _create_table convention where table_kind is a named parameter and the Snowflake adapter derives it from table_format:

• EngineAdapter.clone_table(..., table_format=None, table_kind=None, **kwargs) — kind is now table_kind or "TABLE"
• EngineAdapter.alter_table(alter_expressions, table_format=None)

The Snowflake overrides now match their supertypes exactly, so there's no type: ignore anywhere in the diff.

That signature change meant updating the other adapters that override these methods — bigquery, clickhouse and fabric for alter_table, databricks for clone_table — so they stay Liskov-compatible. They ignore table_format; only Snowflake acts on it. The previous version of this PR missed those, and mypy flagged it.

Tests

Added test_clone_table_iceberg and test_alter_table_iceberg to tests/core/engine_adapter/test_snowflake.py, covering the Iceberg DDL, the non-Iceberg path, and that a non-Snowflake adapter is unaffected by the new argument. Also updated the two test_snapshot_evaluator.py assertions that pin the exact clone_table/alter_table call signatures.

Verification

• tests/core/engine_adapter/ (excluding integration), test_snapshot_evaluator.py and test_schema_diff.py: 926 passed
• Full fast marker suite: no regressions (compared the failure set against pristine main in the same environment — identical)
• mypy: no errors in any of the touched files
• ruff check and ruff format: clean

Integration tests against a live Snowflake account I can't run here, so the generated DDL is verified at the unit level — the emitted statements are CREATE ICEBERG TABLE IF NOT EXISTS ... CLONE ... and ALTER ICEBERG TABLE ... ADD ..., which are the forms Snowflake asks for in the error messages quoted in #5721.

One scoping note: _create_table maps a managed table to DYNAMIC ICEBERG TABLE, but I deliberately didn't extend that to the alter path — dynamic tables aren't schema-migrated through alter_table, and it's outside what #5721 covers. Happy to add it if you'd prefer it handled here.

[sineline]

One adjacent gap I found while auditing the rest of the DDL paths, which I've deliberately left out of this PR — happy to fold it in here or split it into its own issue, whichever you prefer.

SnowflakeEngineAdapter._create_column_comments builds its statement by hand:

combined_sql = f"ALTER {table_kind} {table_sql} ALTER {', '.join(list_comment_sql)}"

table_kind defaults to "TABLE" and no caller passes it for tables (only views pass "VIEW"), so for an Iceberg model this emits ALTER TABLE ... ALTER COLUMN ... COMMENT, which I'd expect Snowflake to reject the same way it rejects the clone and alter statements in #5721.

It's narrower and less severe than the two cases fixed here:

• it only fires from the CTAS path when schema is None (column types not fully known) — otherwise the comments go inline into the CREATE, which already carries ICEBERG
• the call is wrapped in try/except, so it doesn't fail the plan; the symptom is column comments silently not registered, plus a warning that misattributes it to limited permissions

I haven't confirmed the rejection against a live Snowflake account, so treat that as inferred from the error-message pattern rather than verified.

For completeness, the other table DDL paths look correct as they stand and deliberately should not get the keyword:

• DROP TABLE — ICEBERG is optional in DROP [ICEBERG] TABLE
• TRUNCATE TABLE — no ICEBERG variant exists
• ALTER TABLE ... RENAME TO — ALTER ICEBERG TABLE has no RENAME TO clause, so injecting the keyword here would break renames
• DESCRIBE TABLE — works on Iceberg tables

I also checked that stamping the keyword across every expression in alter_table is safe: the operations SQLMesh generates there are add column, drop column, column type change, CLUSTER BY and DROP CLUSTERING KEY, and ALTER ICEBERG TABLE supports all of them.