Skip to content

Commit 9664fca

Browse files
committed
fix: drop clustering key before dropping columns it references
Signed-off-by: Srujan Kumar Gandla <srujankumar.dg@gmail.com>
1 parent 9bd35b7 commit 9664fca

2 files changed

Lines changed: 54 additions & 8 deletions

File tree

‎sqlmesh/core/engine_adapter/mixins.py‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -383,20 +383,27 @@ def get_alter_operations(
383383
self.get_data_objects(target_table_schema, {target_table.name}), 0
384384
)
385385

386+
cluster_key_operation: t.Optional[TableAlterClusterByOperation] = None
386387
if current_table_info and target_table_info:
387388
if target_table_info.is_clustered:
388389
if target_table_info.clustering_key and (
389390
current_table_info.clustering_key != target_table_info.clustering_key
390391
):
391-
operations.append(
392-
TableAlterChangeClusterKeyOperation(
393-
target_table=current_table,
394-
clustering_key=target_table_info.clustering_key,
395-
dialect=self.dialect,
396-
)
392+
cluster_key_operation = TableAlterChangeClusterKeyOperation(
393+
target_table=current_table,
394+
clustering_key=target_table_info.clustering_key,
395+
dialect=self.dialect,
397396
)
398397
elif current_table_info.is_clustered:
399-
operations.append(TableAlterDropClusterKeyOperation(target_table=current_table))
398+
cluster_key_operation = TableAlterDropClusterKeyOperation(
399+
target_table=current_table
400+
)
401+
402+
if cluster_key_operation:
403+
# The clustering key must be dropped (or replaced) before any of the
404+
# columns it references can be dropped: engines like Snowflake reject
405+
# dropping a column that belongs to a clustering key.
406+
operations.insert(0, cluster_key_operation)
400407

401408
return operations
402409

‎tests/core/engine_adapter/test_snowflake.py‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from sqlmesh.core.dialect import normalize_model_name
1111
from sqlmesh.core.engine_adapter import SnowflakeEngineAdapter
1212
from sqlmesh.core.engine_adapter.base import EngineAdapter
13-
from sqlmesh.core.engine_adapter.shared import DataObjectType
13+
from sqlmesh.core.engine_adapter.shared import DataObject, DataObjectType
1414
from sqlmesh.core.model import load_sql_based_model
1515
from sqlmesh.core.model.definition import SqlModel
1616
from sqlmesh.core.node import IntervalUnit
@@ -1049,6 +1049,45 @@ def test_alter_table_iceberg(mocker: MockerFixture, make_mocked_engine_adapter:
10491049
assert to_sql_calls(adapter) == ['ALTER TABLE "test_table" ADD "b" INT']
10501050

10511051

1052+
def test_alter_table_drops_clustering_key_before_drop_column(
1053+
mocker: MockerFixture, make_mocked_engine_adapter: t.Callable
1054+
):
1055+
mocker.patch("sqlmesh.core.engine_adapter.snowflake.SnowflakeEngineAdapter.set_current_catalog")
1056+
adapter = make_mocked_engine_adapter(SnowflakeEngineAdapter, default_catalog="test_catalog")
1057+
1058+
current_table = {"a": "INT", "b": "INT"}
1059+
target_table = {"a": "INT"}
1060+
adapter.columns = lambda table_name, **kwargs: {
1061+
k: exp.DataType.build(v)
1062+
for k, v in (current_table if table_name == "test_table" else target_table).items()
1063+
}
1064+
1065+
def _get_data_objects(schema_name, object_names=None, **kwargs):
1066+
table_name = next(iter(object_names or []))
1067+
return [
1068+
DataObject(
1069+
catalog="test_catalog",
1070+
schema="test_schema",
1071+
name=table_name,
1072+
type=DataObjectType.TABLE,
1073+
# the current table is clustered by the column being dropped
1074+
clustering_key='"b"' if table_name == "test_table" else None,
1075+
)
1076+
]
1077+
1078+
mocker.patch.object(adapter, "get_data_objects", side_effect=_get_data_objects)
1079+
1080+
alter_operations = adapter.get_alter_operations("test_table", "target_table")
1081+
adapter.alter_table(alter_operations)
1082+
1083+
# Snowflake rejects dropping a column that belongs to a clustering key, so the
1084+
# clustering key must be dropped before the column is dropped.
1085+
assert to_sql_calls(adapter) == [
1086+
'ALTER TABLE "test_table" DROP CLUSTERING KEY',
1087+
'ALTER TABLE "test_table" DROP COLUMN "b"',
1088+
]
1089+
1090+
10521091
def test_create_view_with_schema_and_grants(
10531092
snowflake_mocked_engine_adapter: SnowflakeEngineAdapter,
10541093
):

0 commit comments

Comments
 (0)