Skip to content

Commit 80d884f

Browse files
committed
fix(clickhouse): preserve cluster execution for partition alters
Signed-off-by: mday-io <mdaytn@gmail.com>
1 parent 5741bb6 commit 80d884f

2 files changed

Lines changed: 47 additions & 3 deletions

File tree

sqlmesh/core/engine_adapter/clickhouse.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,17 @@ def alter_table(
739739
altered_table = (
740740
alter_expression.this if isinstance(alter_expression.this, exp.Table) else None
741741
)
742-
if self._should_use_on_cluster(altered_table):
742+
# Replicated databases propagate metadata ALTERs, but not partition
743+
# operations. Partition overwrites must still reach every shard.
744+
alters_partitions = any(
745+
isinstance(action, (exp.DropPartition, exp.ReplacePartition))
746+
for action in alter_expression.args.get("actions", [])
747+
)
748+
if (
749+
self.engine_run_mode.is_cluster
750+
if alters_partitions
751+
else self._should_use_on_cluster(altered_table)
752+
):
743753
alter_expression.set(
744754
"cluster", exp.OnCluster(this=exp.to_identifier(self.cluster))
745755
)

tests/core/engine_adapter/test_clickhouse.py

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,8 +1836,12 @@ def test_alter_table_decides_on_cluster_per_expression(
18361836

18371837
adapter.alter_table(
18381838
[
1839-
parse_one("ALTER TABLE a_db.t ADD COLUMN x UInt64", dialect="clickhouse"),
1840-
parse_one("ALTER TABLE r_db.t ADD COLUMN x UInt64", dialect="clickhouse"),
1839+
parse_one("ALTER TABLE a_db.t ADD COLUMN x UInt64", dialect="clickhouse").assert_is(
1840+
exp.Alter
1841+
),
1842+
parse_one("ALTER TABLE r_db.t ADD COLUMN x UInt64", dialect="clickhouse").assert_is(
1843+
exp.Alter
1844+
),
18411845
]
18421846
)
18431847

@@ -1846,6 +1850,36 @@ def test_alter_table_decides_on_cluster_per_expression(
18461850
assert "ON CLUSTER" not in calls[1]
18471851

18481852

1853+
@pytest.mark.parametrize("cluster", [None, "my_cluster"])
1854+
@pytest.mark.parametrize("database_engine", ["Atomic", "Replicated"])
1855+
@pytest.mark.parametrize("operation", ["drop", "replace", "both"])
1856+
def test_alter_partition_keeps_on_cluster(
1857+
make_mocked_engine_adapter: t.Callable, mocker, cluster, database_engine, operation
1858+
):
1859+
adapter = make_mocked_engine_adapter(ClickhouseEngineAdapter, cluster=cluster)
1860+
mocker.patch.object(ClickhouseEngineAdapter, "_has_replicated_database", True)
1861+
mocker.patch.object(ClickhouseEngineAdapter, "_database_engine", return_value=database_engine)
1862+
1863+
# Use the same AST builder as incremental partition overwrites.
1864+
alter = adapter._build_alter_partition_exp(
1865+
exp.to_table("my_db.target"),
1866+
exp.to_table("my_db.source"),
1867+
{"1"} if operation in ("replace", "both") else set(),
1868+
{"2"} if operation in ("drop", "both") else set(),
1869+
)
1870+
adapter.alter_table([alter])
1871+
1872+
actions = []
1873+
if operation in ("replace", "both"):
1874+
actions.append('REPLACE PARTITION ID \'1\' FROM "my_db"."source"')
1875+
if operation in ("drop", "both"):
1876+
actions.append("DROP PARTITION ID '2'")
1877+
cluster_sql = ' ON CLUSTER "my_cluster"' if cluster else ""
1878+
assert to_sql_calls(adapter) == [
1879+
f'ALTER TABLE "my_db"."target"{cluster_sql} {", ".join(actions)}'
1880+
]
1881+
1882+
18491883
def test_cross_engine_rename_and_exchange_are_refused(
18501884
make_mocked_engine_adapter: t.Callable, mocker
18511885
):

0 commit comments

Comments
 (0)