fix(clickhouse): suppress ON CLUSTER for object DDL in a Replicated database

[mday-io]

Description

Fix ClickHouse error 80 when SQLMesh creates or alters objects in a Replicated database with a cluster configured.

• Omit ON CLUSTER where the database already propagates metadata changes.
• Keep database creation/removal and partition DROP/REPLACE operations cluster-wide.
• Preserve existing behavior for Atomic databases and failed lookups; reject rename/exchange operations between databases with different replication policies.
• Retry failed metadata lookups on the next operation, and cache successful results.

Test Plan

• make style passes.
• make fast-test: 2,691 tests passed in the main batch; all 196 isolation tests passed (5 isolated, 1 registry, 190 dialect). Skips are recorded per batch in the local logs.
• All 68 ClickHouse adapter tests pass, including 12 partition regression cases and three metadata-recovery cases. The recovery cases fail before this fix and pass afterward; they also verify successful cache reuse.
• Independent review passed for the conflict resolution and metadata-recovery fix.
• Earlier live test on ClickHouse 26.6.3.62: table creation fails before the fix and succeeds across three replicas afterward. Not rerun for this update.
• All required CI checks pass.

Checklist

• I have run make style and fixed any issues
• I have added tests for my changes (if applicable)
• All existing tests pass (make fast-test)
• My commits are signed off (git commit -s) per the DCO

[mday-io]

ready for review

[nicksspirit]

Will look into this one

[nicksspirit]

Hi @mday-io — thanks for tackling the error 80 on Replicated databases. Checking the target database's engine per statement, and keeping ON CLUSTER for CREATE/DROP DATABASE, looks right to me, and the tests cover it well.

Two small things:

The three except Exception blocks in the engine lookups return silently:

sqlmesh/sqlmesh/core/engine_adapter/clickhouse.py

Line 1141 in 3f397d7

except Exception:

. If the lookup fails, for example a user without access to system.databases, the fix quietly turns off and error 80 comes back with no clue why. Could you add a logger.debug or logger.warning there?

The ClickHouse docs say SQLMesh injects ON CLUSTER into all DDL (

sqlmesh/docs/integrations/engines/clickhouse.md

Line 52 in 3f397d7

If you provide a cluster name in your SQLMesh connection configuration, SQLMesh will automatically inject the `ON CLUSTER` statement into the DDL commands for all objects created while executing the project. We provide more information about clusters in SQLMesh [below](#cluster-specification).

, and line 99). Could you add a sentence saying it's skipped for Replicated databases?

One question, and this is already on main, not from this PR. Partition ALTERs still get ON CLUSTER:

sqlmesh/sqlmesh/core/engine_adapter/clickhouse.py

Lines 776 to 787 in 3f397d7

	alters_partitions = any(
	isinstance(action, (exp.DropPartition, exp.ReplacePartition))
	for action in alter_expression.args.get("actions", [])
	)
	if (
	self.engine_run_mode.is_cluster
	if alters_partitions
	else self._should_use_on_cluster(altered_table)
	):
	alter_expression.set(
	"cluster", exp.OnCluster(this=exp.to_identifier(self.cluster))
	)

. SQLMesh emits REPLACE PARTITION and DROP PARTITION in one ALTER. As far as I can tell from shouldReplicateQuery in the ClickHouse source (https://github.com/ClickHouse/ClickHouse/blob/master/src/Databases/DatabaseReplicated.cpp), the exemption only applies when every command in the ALTER is the same kind, so a mixed one is still replicated by the database. On a multi-shard Replicated database that should still hit error 80. Was your live test single-shard? If so, it may be worth a note, or a follow-up.

Thanks again for taking the time on this.

[mday-io]

Added warnings for all three metadata lookup failures in 92864ad76. Each warning identifies the failed lookup and includes the exception. The existing ON CLUSTER fallback and subsequent retry remain unchanged.

Both docs claims now explain that metadata DDL targeting Replicated databases skips ON CLUSTER. They also clarify that database creation/removal and partition operations retain it.

On the topology question, the original commit describes the 26.6.3.62 run as one shard with three replicas. The retained PR result records three replicas without explicitly recording the shard count. Neither provides multi-shard mixed-partition validation. Your concern is supported by ClickHouse's source and predates this PR. The 26.6.3.62 implementation also lacks the newer REPLACE-only exemption, so splitting the mixed ALTER alone is not an established fix on that version. I added a documentation caveat and recorded a version-aware partition validation follow-up. No live multi-shard reproduction was performed for this update.

Validation passed. All three diagnostic regression cases failed before the fix and passed afterward. The complete ClickHouse adapter module passed 68 tests. make style passed, and all four make fast-test batches passed with 2,887 tests and 13 skips. New-head CI is separate from these local results.

[nicksspirit]

Thanks @mday-io, the warnings and the docs caveat look good, and the test checking the exact message per failure is a nice touch. Appreciate the honest answer on the multi-shard case and the follow-up.

One small thing: could you switch the three new warnings to f-strings? The engine adapters mostly use them, for example logger.info(f"Loading dataframe to BigQuery. Table Path: {table.path}") in bigquery.py. The three are here:

sqlmesh/sqlmesh/core/engine_adapter/clickhouse.py

Line 1141 in 939da1f

logger.warning(

, L1169 and L1196. For example:

logger.warning(f"Failed to determine engine for ClickHouse database '{database}'; keeping ON CLUSTER: {ex}")

The tests compare the exact message text, so they should still pass once the text matches.

The repo mixes both styles today, so this isn't something you could have known. I'll put up a separate PR to add a lint rule so only f-strings are allowed from here on.

Everything else looks good to me. Thanks again!

[mday-io]

Switched all three warnings to f-strings in 658ab8b8f, preserving the exact message text. The existing three message/recovery cases, make style, and all four make fast-test batches passed (2,891 tests, 13 skips). Independent review found no issues.