Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions sqlmesh/core/engine_adapter/clickhouse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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()
]
)

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
35 changes: 34 additions & 1 deletion tests/core/engine_adapter/test_clickhouse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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(
Expand Down
Loading