fix(clickhouse): combine physical_properties into one SETTINGS clause (#5817)

Signed-off-by: mokashang <shangmengjiajiajia@gmail.com>

Author: mokashang

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(

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(