diff --git a/sqlmesh/core/engine_adapter/clickhouse.py b/sqlmesh/core/engine_adapter/clickhouse.py index 698b2f4128..c41681ade2 100644 --- a/sqlmesh/core/engine_adapter/clickhouse.py +++ b/sqlmesh/core/engine_adapter/clickhouse.py @@ -716,8 +716,12 @@ def use_server_nulls_for_unmatched_after_join( return query def _build_settings_property( - self, key: str, value: exp.Expr | str | int | float + self, settings: t.Mapping[str, exp.Expr | str | int | float] ) -> exp.SettingsProperty: + # ClickHouse requires every key=value pair to live under a single + # SETTINGS clause (`SETTINGS a = 1, b = 2`). Emitting one + # SettingsProperty per pair produces repeated SETTINGS keywords and a + # syntax error at execution time. return exp.SettingsProperty( expressions=[ exp.EQ( @@ -726,6 +730,7 @@ def _build_settings_property( if isinstance(value, exp.Expr) else exp.Literal(this=value, is_string=isinstance(value, str)), ) + for key, value in settings.items() ] ) @@ -827,9 +832,7 @@ def _build_table_properties_exp( properties.append(exp.EmptyProperty()) if table_properties_copy: - properties.extend( - [self._build_settings_property(k, v) for k, v in table_properties_copy.items()] - ) + properties.append(self._build_settings_property(table_properties_copy)) if table_description: properties.append( @@ -858,9 +861,7 @@ def _build_view_properties_exp( properties.append(exp.OnCluster(this=exp.to_identifier(self.cluster))) if view_properties_copy: - properties.extend( - [self._build_settings_property(k, v) for k, v in view_properties_copy.items()] - ) + properties.append(self._build_settings_property(view_properties_copy)) if table_description: properties.append( diff --git a/tests/core/engine_adapter/test_clickhouse.py b/tests/core/engine_adapter/test_clickhouse.py index 7ff971b742..b2ff0592d2 100644 --- a/tests/core/engine_adapter/test_clickhouse.py +++ b/tests/core/engine_adapter/test_clickhouse.py @@ -316,13 +316,32 @@ def build_properties_sql(storage_format="", order_by="", primary_key="", propert == "ENGINE=MergeTree ORDER BY (a, b + 1) PRIMARY KEY (a, b)" ) + # Multiple physical_properties must be combined into a single comma-separated + # SETTINGS clause. ClickHouse rejects repeated SETTINGS keywords with a syntax + # error (see https://github.com/SQLMesh/sqlmesh/issues/5803). assert ( build_properties_sql( order_by="ORDER_BY = (a, b + 1),", primary_key="PRIMARY_KEY = (a, b),", properties="PROP1 = 1, PROP2 = '2'", ) - == "ENGINE=MergeTree ORDER BY (a, b + 1) PRIMARY KEY (a, b) SETTINGS prop1 = 1 SETTINGS prop2 = '2'" + == "ENGINE=MergeTree ORDER BY (a, b + 1) PRIMARY KEY (a, b) SETTINGS prop1 = 1, prop2 = '2'" + ) + + # Regression test for #5803: three or more SETTINGS entries also combine. + assert ( + build_properties_sql( + order_by="ORDER_BY = (orders_id),", + properties=( + "min_age_to_force_merge_seconds = 3600, " + "min_age_to_force_merge_on_partition_only = 1, " + "index_granularity = 8192" + ), + ) + == "ENGINE=MergeTree ORDER BY (orders_id) " + "SETTINGS min_age_to_force_merge_seconds = 3600, " + "min_age_to_force_merge_on_partition_only = 1, " + "index_granularity = 8192" ) assert ( @@ -345,6 +364,20 @@ def build_properties_sql(storage_format="", order_by="", primary_key="", propert ) +def test_view_properties_combine_settings(adapter: ClickhouseEngineAdapter): + # View properties hit the same SettingsProperty code path as table + # properties (#5803): multiple entries must collapse into one SETTINGS + # clause rather than emit repeated SETTINGS keywords. + view_properties_exp = adapter._build_view_properties_exp( + view_properties={ + "prop1": exp.Literal.number(1), + "prop2": exp.Literal.string("2"), + } + ) + assert view_properties_exp is not None + assert view_properties_exp.sql("clickhouse") == "SETTINGS prop1 = 1, prop2 = '2'" + + def test_partitioned_by_expr(make_mocked_engine_adapter: t.Callable): # user doesn't specify, unknown time column type model = load_sql_based_model(