From ecc4784c8d84b4f27f7710a48b9e95c80c518dcc Mon Sep 17 00:00:00 2001 From: mday-io Date: Thu, 6 Aug 2026 18:29:34 +0000 Subject: [PATCH 1/8] fix(format): render warehouse-SQL header properties with the model dialect #5864 stopped transpiling MODEL/AUDIT/METRIC headers so that SQLMesh's own boolean properties would survive formatting -- on tsql, `allow_partials TRUE` was being rewritten to `(1 = 1)`, which then fails to parse at all and leaves the model file broken. That fix rendered the entire header generically, including the properties whose values are the user's warehouse SQL. Those lose their dialect: `columns (ts DATETIME2(6))` becomes `TIMESTAMP(6)`, and an audit argument such as `CAST('2024-01-01' AS DATETIME2)` is silently downgraded the same way. Split the header per property instead of per expression. The split is derived from the field declarations themselves: expression-typed fields (columns, audits, signals, partitioned_by, physical_properties, ...) hold warehouse SQL and render with the model dialect, while scalar-typed fields (allow_partials, description, kind, ...) are SQLMesh's own semantics and stay dialect-agnostic. Deriving it means the policy stays correct as properties are added, and a field that is missed fails safe -- a keyword is not canonicalized, rather than a user's SQL being corrupted. Covers MODEL, AUDIT and METRIC headers, and the expression properties nested inside `kind` such as `time_data_type` and `unique_key`. Signed-off-by: mday-io --- sqlmesh/core/dialect.py | 94 +++++++++++++++++++++++++++++++-- tests/core/test_dialect.py | 103 +++++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 3 deletions(-) diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py index e4ab522198..d2f6fffd13 100644 --- a/sqlmesh/core/dialect.py +++ b/sqlmesh/core/dialect.py @@ -734,6 +734,52 @@ def parse(self: Parser) -> t.Optional[exp.Expr]: } +_SQLMESH_META_DIALECT = "sqlmesh_meta_dialect" + + +def _holds_expression(annotation: t.Any) -> bool: + """Whether a declared field type bottoms out in a SQLGlot expression. + + Covers List[exp.Expr], Optional[Dict[str, exp.DataType]], Optional[exp.Tuple] and + the nested Tuple[str, Dict[str, exp.Expr]] shape used by audits/signals. + """ + if isinstance(annotation, type) and issubclass(annotation, exp.Expr): + return True + return any(_holds_expression(arg) for arg in t.get_args(annotation)) + + +@functools.lru_cache(maxsize=1) +def _meta_render_policy() -> t.Dict[str, bool]: + """Map header property name -> whether its value is warehouse SQL. + + Derived from the field declarations themselves, so it stays correct as properties + are added: expression-typed values (columns, audits, physical_properties, ...) are + the user's warehouse SQL and must render in the model's dialect, while scalar-typed + values (allow_partials, description, kind, ...) are SQLMesh's own semantics and must + stay dialect-agnostic -- transpiling those is what corrupts `allow_partials TRUE` + into tsql's unparseable `(1 = 1)`. + """ + import inspect + + from sqlmesh.core.audit.definition import ModelAudit + from sqlmesh.core.metric.definition import MetricMeta + from sqlmesh.core.model import kind as kind_module + from sqlmesh.core.model.meta import ModelMeta + + sources: t.List[t.Any] = [ModelMeta, ModelAudit, MetricMeta] + sources.extend( + obj + for name, obj in vars(kind_module).items() + if inspect.isclass(obj) and hasattr(obj, "model_fields") and name.endswith("Kind") + ) + + policy: t.Dict[str, bool] = {} + for source in sources: + for name, field in source.model_fields.items(): + policy.setdefault((field.alias or name).lower(), _holds_expression(field.annotation)) + return policy + + def _props_sql(self: Generator, expressions: t.List[exp.Expr]) -> str: props = [] size = len(expressions) @@ -742,7 +788,31 @@ def _props_sql(self: Generator, expressions: t.List[exp.Expr]) -> str: if isinstance(prop, MacroFunc): sql = self.indent(self.sql(prop, comment=False)) else: - sql = self.indent(f"{prop.name} {self.sql(prop, 'value')}") + value = prop.args.get("value") + parent = prop.parent + meta_dialect = parent.meta.get(_SQLMESH_META_DIALECT) if parent else None + + if ( + meta_dialect + and isinstance(value, exp.Expr) + and _meta_render_policy().get(prop.name.lower()) + ): + value_sql = value.sql( + dialect=meta_dialect, + pretty=self.pretty, + identify=self.identify, + normalize=self.normalize, + pad=self.pad, + indent=self._indent, + normalize_functions=self.normalize_functions, + leading_comma=self.leading_comma, + max_text_width=self.max_text_width, + comments=self.comments, + ) + else: + value_sql = self.sql(prop, "value") + + sql = self.indent(f"{prop.name} {value_sql}") if i < size - 1: sql += "," @@ -853,11 +923,29 @@ def format_model_expressions( Returns: A string representing the formatted model. """ + + def tag_meta_dialect(expression: exp.Expr) -> exp.Expr: + """Record the model dialect on meta nodes so `_props_sql` can render the + warehouse-SQL properties (columns, audits, physical_properties, ...) with it + while the SQLMesh-owned ones stay dialect-agnostic. Tags nested ModelKind + nodes too, since kinds carry expression properties of their own such as + `time_data_type` and `unique_key`.""" + if not dialect or not is_meta_expression(expression): + return expression + + expression = expression.copy() + for node in expression.find_all(Model, Audit, Metric, ModelKind): + node.meta[_SQLMESH_META_DIALECT] = dialect + expression.meta[_SQLMESH_META_DIALECT] = dialect + return expression + if len(expressions) == 1 and is_meta_expression(expressions[0]): # Meta expressions (MODEL/AUDIT/METRIC) are SQLMesh DDL, not standard SQL, # so they must never be transpiled to the target dialect (e.g. tsql would # rewrite a boolean property like `allow_partials TRUE` to `(1 = 1)`). - return expressions[0].sql( + # Individual properties whose values *are* warehouse SQL still render with + # the model dialect -- see `_props_sql` / `_meta_render_policy`. + return tag_meta_dialect(expressions[0]).sql( pretty=True, dialect=None, normalize_functions=normalize_functions ) @@ -893,7 +981,7 @@ def cast_to_colon(node: exp.Expr) -> exp.Expr: return ";\n\n".join( # Meta expressions (MODEL/AUDIT/METRIC) are SQLMesh DDL and must stay # dialect-agnostic; only the actual query/statement expressions transpile. - expression.sql( + tag_meta_dialect(expression).sql( pretty=True, dialect=None if is_meta_expression(expression) else dialect, normalize_functions=normalize_functions, diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index 142b40b31f..999e205af7 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -342,6 +342,109 @@ def test_format_model_expressions(): ) +@pytest.mark.parametrize( + "dialect,audit_type,int_type", + # These dialects spell the same types differently -- fabric renders a bare DATETIME2 + # with its default precision and keeps INT, tsql does the reverse. The point of the + # test is that each keeps *its own* spelling rather than being flattened. + [("tsql", "DATETIME2", "INTEGER"), ("fabric", "DATETIME2(6)", "INT")], +) +def test_format_model_expressions_meta_render_policy(dialect: str, audit_type: str, int_type: str): + """Header properties whose values are warehouse SQL render with the model dialect, + while SQLMesh's own properties stay dialect-agnostic. + + Rendering the whole header with the dialect corrupts SQLMesh DDL (tsql turns + `allow_partials TRUE` into the unparseable `(1 = 1)`), but rendering all of it + generically discards dialect-specific values the user authored, such as the + `DATETIME2` types below. The split is derived from the field declarations, so it + covers `columns`, `audits`, `physical_properties` and the expression properties + nested inside `kind` alike. + """ + formatted = format_model_expressions( + parse( + f""" + MODEL ( + name a.b, + dialect {dialect}, + kind SCD_TYPE_2_BY_TIME ( + unique_key id, + time_data_type DATETIME2(6) + ), + allow_partials true, + description 'my description', + columns ( + ts DATETIME2(6) + ), + audits ( + my_audit(threshold := CAST('2024-01-01' AS DATETIME2)) + ), + physical_properties ( + labels = (('env', 'prod')) + ) + ); + + SELECT CAST(x AS INT) AS y FROM t + """ + ), + dialect=dialect, + ) + + assert ( + formatted + == f"""MODEL ( + name a.b, + dialect {dialect}, + kind SCD_TYPE_2_BY_TIME ( + unique_key id, + time_data_type DATETIME2(6) + ), + allow_partials TRUE, + description 'my description', + columns ( + ts DATETIME2(6) + ), + audits ( + my_audit(threshold := '2024-01-01'::{audit_type}) + ), + physical_properties ( + labels = ( + ('env', 'prod') + ) + ) +); + +SELECT + x::{int_type} AS y +FROM t""" + ) + + +def test_format_audit_expressions_meta_render_policy(): + """AUDIT headers have their own meta model, and get the same split: `blocking` is + SQLMesh's own boolean and must not become tsql's `(1 = 0)`, while `defaults` holds + user expressions and keeps its dialect-specific type.""" + formatted = format_model_expressions( + parse( + """ + AUDIT ( + name my_audit, + dialect tsql, + blocking false, + defaults ( + cutoff := CAST('2024-01-01' AS DATETIME2) + ) + ); + + SELECT * FROM t WHERE x > 0 + """ + ), + dialect="tsql", + ) + + assert "blocking FALSE" in formatted + assert "cutoff := '2024-01-01'::DATETIME2" in formatted + + def test_format_model_expressions_normalize_functions(): """Regression: formatter function-name casing behavior. From 0a71611db69eb30ae25082c2d87cb7e08724d2b3 Mon Sep 17 00:00:00 2001 From: mday-io Date: Thu, 6 Aug 2026 21:32:40 +0000 Subject: [PATCH 2/8] test(format): assert header formatting is idempotent Rendering a dialect-specific type with the generic generator compounds across runs rather than merely looking different: tsql `DATETIME2` renders as `TIMESTAMP`, and tsql parses `TIMESTAMP` as ROWVERSION, so a second pass writes `VARBINARY`. Two runs of `sqlmesh format` silently turned a datetime into a binary type -- and for an SCD kind's `time_data_type` that is the physical type of the valid_from/valid_to columns. Covers columns, audits, nested kind properties, physical_properties and the SQLMesh-owned scalars. Signed-off-by: mday-io --- tests/core/test_dialect.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index 999e205af7..870e98e64d 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -419,6 +419,33 @@ def test_format_model_expressions_meta_render_policy(dialect: str, audit_type: s ) +@pytest.mark.parametrize( + "header", + [ + "columns (ts DATETIME2(6))", + "audits (my_audit(t := CAST('2024-01-01' AS DATETIME2)))", + "kind SCD_TYPE_2_BY_COLUMN(unique_key id, columns (a, b), time_data_type DATETIME2(6))", + "physical_properties (labels = (('env', 'prod')))", + "allow_partials true, description 'my description'", + ], +) +def test_format_model_expressions_is_idempotent(header: str): + """Formatting an already-formatted model must be a no-op. + + Rendering a dialect-specific type with the generic generator does not merely lose + formatting, it compounds: tsql `DATETIME2` renders as `TIMESTAMP`, and tsql parses + `TIMESTAMP` as ROWVERSION (a binary type), so a second pass writes `VARBINARY`. Two + runs of `sqlmesh format` silently turned a datetime into a binary type -- and for + `time_data_type` that is the physical type of the SCD valid_from/valid_to columns. + """ + source = f"MODEL (name a.b, dialect tsql, {header});\nSELECT 1 AS x" + + once = format_model_expressions(parse(source, default_dialect="tsql"), dialect="tsql") + twice = format_model_expressions(parse(once, default_dialect="tsql"), dialect="tsql") + + assert once == twice + + def test_format_audit_expressions_meta_render_policy(): """AUDIT headers have their own meta model, and get the same split: `blocking` is SQLMesh's own boolean and must not become tsql's `(1 = 0)`, while `defaults` holds From f67fd27349ef952f96ae5b990e6c112f984f396b Mon Sep 17 00:00:00 2001 From: mday-io Date: Fri, 7 Aug 2026 12:48:07 +0000 Subject: [PATCH 3/8] fix(format): render macro properties in the header with the model dialect A macro in property position wraps user-authored arguments, so it carries warehouse SQL the same way `columns` or `audits` do. It took a separate branch in _props_sql and kept rendering generically, which left it on the compounding path: DATETIME2 -> TIMESTAMP -> VARBINARY across two format runs. Signed-off-by: mday-io --- sqlmesh/core/dialect.py | 42 ++++++++++++++++++++++++-------------- tests/core/test_dialect.py | 1 + 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py index d2f6fffd13..4d66ad2a8c 100644 --- a/sqlmesh/core/dialect.py +++ b/sqlmesh/core/dialect.py @@ -785,30 +785,42 @@ def _props_sql(self: Generator, expressions: t.List[exp.Expr]) -> str: size = len(expressions) for i, prop in enumerate(expressions): + parent = prop.parent + meta_dialect = parent.meta.get(_SQLMESH_META_DIALECT) if parent else None + + def render_with_model_dialect(node: exp.Expr, **overrides: t.Any) -> str: + opts: t.Dict[str, t.Any] = { + "dialect": meta_dialect, + "pretty": self.pretty, + "identify": self.identify, + "normalize": self.normalize, + "pad": self.pad, + "indent": self._indent, + "normalize_functions": self.normalize_functions, + "leading_comma": self.leading_comma, + "max_text_width": self.max_text_width, + "comments": self.comments, + } + opts.update(overrides) + return node.sql(**opts) + if isinstance(prop, MacroFunc): - sql = self.indent(self.sql(prop, comment=False)) + # A macro in property position wraps user-authored arguments, so it carries + # warehouse SQL the same way `columns` or `audits` do. + sql = self.indent( + render_with_model_dialect(prop, comments=False) + if meta_dialect + else self.sql(prop, comment=False) + ) else: value = prop.args.get("value") - parent = prop.parent - meta_dialect = parent.meta.get(_SQLMESH_META_DIALECT) if parent else None if ( meta_dialect and isinstance(value, exp.Expr) and _meta_render_policy().get(prop.name.lower()) ): - value_sql = value.sql( - dialect=meta_dialect, - pretty=self.pretty, - identify=self.identify, - normalize=self.normalize, - pad=self.pad, - indent=self._indent, - normalize_functions=self.normalize_functions, - leading_comma=self.leading_comma, - max_text_width=self.max_text_width, - comments=self.comments, - ) + value_sql = render_with_model_dialect(value) else: value_sql = self.sql(prop, "value") diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index 870e98e64d..c2e2b6d750 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -427,6 +427,7 @@ def test_format_model_expressions_meta_render_policy(dialect: str, audit_type: s "kind SCD_TYPE_2_BY_COLUMN(unique_key id, columns (a, b), time_data_type DATETIME2(6))", "physical_properties (labels = (('env', 'prod')))", "allow_partials true, description 'my description'", + "@my_prop(cutoff := CAST('2024-01-01' AS DATETIME2))", ], ) def test_format_model_expressions_is_idempotent(header: str): From e6cb87b03a23c5dec3864ef60db78961b38e9101 Mon Sep 17 00:00:00 2001 From: mday-io Date: Sun, 9 Aug 2026 19:04:15 +0000 Subject: [PATCH 4/8] fix(format): recurse into nested Pydantic models and preserve macro property comments Two gaps in the header-property dialect-render policy from the previous fix: - `_holds_expression` only checked the outer type annotation and typing generics (`Optional`, `List`, ...), so a nested Pydantic model wrapping an expression field -- `TimeColumn` on `IncrementalByTimeRangeKind.time_column` -- was misclassified as a scalar property and fell back to generic rendering, losing dialect-specific identifier quoting (tsql `[end]` became ANSI `"end"`). Recurse into `model_fields` for any type that exposes them, guarded by a visited set. - The `MacroFunc` dialect-render branch passed `comments=False` into `render_with_model_dialect`, which threads it to `Expression.sql()`'s fresh per-call `Generator` constructor -- a generator-wide flag that disables every comment in the subtree, not just the redundant outer `maybe_comment` call. Comments inside macro header-properties (e.g. `@my_prop(cutoff := ... /* note */)`) were silently dropped whenever the model declared a `dialect`. Render a copy of the property with its own top-level comments cleared instead, leaving `.this`'s comments -- which `_macro_func_sql` already attaches -- untouched. Signed-off-by: mday-io --- sqlmesh/core/dialect.py | 44 ++++++++++++++----- tests/core/test_dialect.py | 89 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 12 deletions(-) diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py index 4d66ad2a8c..d6af7edb7d 100644 --- a/sqlmesh/core/dialect.py +++ b/sqlmesh/core/dialect.py @@ -737,15 +737,28 @@ def parse(self: Parser) -> t.Optional[exp.Expr]: _SQLMESH_META_DIALECT = "sqlmesh_meta_dialect" -def _holds_expression(annotation: t.Any) -> bool: +def _holds_expression(annotation: t.Any, _visited: t.Optional[t.FrozenSet[t.Any]] = None) -> bool: """Whether a declared field type bottoms out in a SQLGlot expression. - Covers List[exp.Expr], Optional[Dict[str, exp.DataType]], Optional[exp.Tuple] and - the nested Tuple[str, Dict[str, exp.Expr]] shape used by audits/signals. + Covers List[exp.Expr], Optional[Dict[str, exp.DataType]], Optional[exp.Tuple], the + nested Tuple[str, Dict[str, exp.Expr]] shape used by audits/signals, and nested + Pydantic models that themselves wrap an expression field, such as `TimeColumn` + (IncrementalByTimeRangeKind.time_column). """ - if isinstance(annotation, type) and issubclass(annotation, exp.Expr): - return True - return any(_holds_expression(arg) for arg in t.get_args(annotation)) + if isinstance(annotation, type): + if issubclass(annotation, exp.Expr): + return True + visited = _visited or frozenset() + if annotation in visited: + return False + if hasattr(annotation, "model_fields"): + visited = visited | {annotation} + return any( + _holds_expression(field.annotation, visited) + for field in annotation.model_fields.values() + ) + return False + return any(_holds_expression(arg, _visited) for arg in t.get_args(annotation)) @functools.lru_cache(maxsize=1) @@ -806,12 +819,19 @@ def render_with_model_dialect(node: exp.Expr, **overrides: t.Any) -> str: if isinstance(prop, MacroFunc): # A macro in property position wraps user-authored arguments, so it carries - # warehouse SQL the same way `columns` or `audits` do. - sql = self.indent( - render_with_model_dialect(prop, comments=False) - if meta_dialect - else self.sql(prop, comment=False) - ) + # warehouse SQL the same way `columns` or `audits` do. Clear the outer node's + # own comments (not `.this`'s, which `_macro_func_sql` already attaches) + # before rendering with the model dialect, mirroring what `comment=False` + # does for the non-dialect path below -- passing `comments=False` here + # instead would build a fresh Generator with comments globally disabled, + # silently dropping every comment in the subtree rather than just the + # redundant outer one. + if meta_dialect: + prop_for_render = prop.copy() + prop_for_render.comments = None + sql = self.indent(render_with_model_dialect(prop_for_render)) + else: + sql = self.indent(self.sql(prop, comment=False)) else: value = prop.args.get("value") diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index c2e2b6d750..0ef9666cf3 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -473,6 +473,95 @@ def test_format_audit_expressions_meta_render_policy(): assert "cutoff := '2024-01-01'::DATETIME2" in formatted +def test_format_model_expressions_time_column_dialect(): + """`time_column` is a nested Pydantic model (`TimeColumn`) wrapping an expression, not + an `exp.Expr` annotation itself, so the render-policy reflection must recurse into + nested Pydantic models to classify it as warehouse SQL. Otherwise it falls back to + generic rendering and loses dialect-specific identifier quoting: tsql's `[end]` + becomes ANSI `"end"`, even though the same identifier in the query body is correctly + kept as `[end]`. + """ + formatted = format_model_expressions( + parse( + """ + MODEL ( + name a.b, + dialect tsql, + kind INCREMENTAL_BY_TIME_RANGE ( + time_column [end] + ) + ); + + SELECT 1 AS x, [end] FROM t + """, + default_dialect="tsql", + ), + dialect="tsql", + ) + + assert ( + formatted + == """MODEL ( + name a.b, + dialect tsql, + kind INCREMENTAL_BY_TIME_RANGE ( + time_column [end] + ) +); + +SELECT + 1 AS x, + [end] +FROM t""" + ) + + +def test_format_model_expressions_macro_property_comments_preserved_with_dialect(): + """Comments inside a macro header-property must survive formatting when the model + has a `dialect` set. + + The dialect-render path goes through `Expression.sql(dialect=...)`, which builds a + fresh `Generator` with `comments` as a constructor flag: passing `comments=False` + there disables comment rendering for the *entire* subtree, rather than just + suppressing the redundant outer-level `maybe_comment` call the way `comment=False` + does for `Generator.sql()`. That previously caused comments like `/* inline note */` + to be silently dropped whenever the model declared a `dialect`. + """ + formatted = format_model_expressions( + parse( + """ + MODEL ( + name a.b, + dialect tsql, + @my_prop(cutoff := CAST('2024-01-01' AS DATETIME2) /* inline note */) + ); + + SELECT 1 AS x + """, + default_dialect="tsql", + ), + dialect="tsql", + ) + + assert "/* inline note */" in formatted + assert ( + formatted + == """MODEL ( + name a.b, + dialect tsql, + @my_prop(cutoff := '2024-01-01'::DATETIME2 /* inline note */) +); + +SELECT + 1 AS x""" + ) + + # Idempotency: formatting an already-formatted macro property must not duplicate or + # drop the comment on a second pass. + twice = format_model_expressions(parse(formatted, default_dialect="tsql"), dialect="tsql") + assert formatted == twice + + def test_format_model_expressions_normalize_functions(): """Regression: formatter function-name casing behavior. From 157e38b765a82d6fba7c3ad42366c987c5be65d3 Mon Sep 17 00:00:00 2001 From: mday-io Date: Sun, 9 Aug 2026 20:51:16 +0000 Subject: [PATCH 5/8] fix(format): stop kind's own render policy from transpiling scalar siblings Recursing _holds_expression into nested Pydantic models to correctly classify TimeColumn (IncrementalByTimeRangeKind.time_column) as warehouse SQL had the side effect of also matching ModelMeta.kind itself, since some member of the ModelKind union holds an expression field. That routed the entire kind (...) subtree through a dialect-specific generator, so on tsql a scalar sibling like forward_only TRUE was rewritten to (1 = 1) -- which reparses fine but silently evaluates to False on reload via str_to_bool. kind's own nested properties are already independently dialect-tagged via the ModelKind expression node's own meta when _props_sql recurses into them, so the outer kind property's policy should never route its subtree through render_with_model_dialect. Stop _holds_expression at _ModelKind subclasses to restore that. Signed-off-by: mday-io --- sqlmesh/core/dialect.py | 14 ++++++++++ tests/core/test_dialect.py | 55 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py index d6af7edb7d..ecd43a566f 100644 --- a/sqlmesh/core/dialect.py +++ b/sqlmesh/core/dialect.py @@ -744,10 +744,24 @@ def _holds_expression(annotation: t.Any, _visited: t.Optional[t.FrozenSet[t.Any] nested Tuple[str, Dict[str, exp.Expr]] shape used by audits/signals, and nested Pydantic models that themselves wrap an expression field, such as `TimeColumn` (IncrementalByTimeRangeKind.time_column). + + Stops at `_ModelKind` subclasses without recursing into their fields: a `kind` + property's own nested properties are independently dialect-tagged via the + `ModelKind` expression node's own meta when `_props_sql` recurses into them, so + treating the `kind` field itself as "holds an expression" -- true only because some + other member of the `ModelKind` union has an expression field, e.g. + `IncrementalByTimeRangeKind.time_column` -- would route its entire subtree, + including scalar sibling properties like `forward_only`, through a dialect-specific + generator and transpile them when they shouldn't be (tsql booleans becoming + `(1 = 1)`, which silently reparses as `False`). """ + from sqlmesh.core.model.kind import _ModelKind + if isinstance(annotation, type): if issubclass(annotation, exp.Expr): return True + if issubclass(annotation, _ModelKind): + return False visited = _visited or frozenset() if annotation in visited: return False diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index 0ef9666cf3..c57ed7e8c4 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -516,6 +516,61 @@ def test_format_model_expressions_time_column_dialect(): ) +def test_format_model_expressions_kind_scalar_sibling_dialect(): + """A scalar sibling property of an expression-bearing property inside `kind` (e.g. + `forward_only` next to `time_column`) must stay dialect-agnostic even though the + render policy correctly marks `kind` as containing an expression-holding field + somewhere in the `ModelKind` union. + + Regression: recursing into nested Pydantic models to fix `time_column` (see + `test_format_model_expressions_time_column_dialect`) made `_holds_expression` also + match on `kind` itself, since *some* member of the `ModelKind` union + (`IncrementalByTimeRangeKind.time_column`) holds an expression. That routed the + entire `kind (...)` subtree through a dialect-specific generator, so tsql's + boolean-literal preprocessing rewrote `forward_only TRUE` into `forward_only (1 = 1)`. + That reparses without error, but `str_to_bool` on `Paren(EQ(1, 1)).name` (`""`) + evaluates to `False`, so the value silently flips on reload. + """ + formatted = format_model_expressions( + parse( + """ + MODEL ( + name a.b, + dialect tsql, + kind INCREMENTAL_BY_TIME_RANGE ( + time_column [end], + forward_only true + ) + ); + + SELECT 1 AS x, [end] FROM t + """, + default_dialect="tsql", + ), + dialect="tsql", + ) + + assert ( + formatted + == """MODEL ( + name a.b, + dialect tsql, + kind INCREMENTAL_BY_TIME_RANGE ( + time_column [end], + forward_only TRUE + ) +); + +SELECT + 1 AS x, + [end] +FROM t""" + ) + + model = load_sql_based_model(parse(formatted, default_dialect="tsql"), dialect="tsql") + assert model.kind.forward_only is True + + def test_format_model_expressions_macro_property_comments_preserved_with_dialect(): """Comments inside a macro header-property must survive formatting when the model has a `dialect` set. From 11d2e628d39eed354c8869869509b146969712d2 Mon Sep 17 00:00:00 2001 From: mday-io Date: Thu, 24 Sep 2026 16:19:33 +0000 Subject: [PATCH 6/8] fix(format): render header list properties as dialect-native arrays Dialect-agnostic header properties that hold a list (`tags`, `ignored_rules`) were rendered by the base generator as `ARRAY(...)`. On BigQuery `ARRAY(` is a subquery constructor, so a multi-element `tags ['a', 'b']` reformatted to `ARRAY('a', 'b')` failed to reload. Render these as `[...]` when the model dialect itself spells arrays with brackets; dialects that use `[`/`]` for identifier quoting (tsql, sqlite, ...) keep `ARRAY(...)`. Elements stay dialect-agnostic. Also give `grain` and `table_properties` the render policy of `grains` / `physical_properties`: they are renamed in `ModelMeta._pre_root_validator` rather than via Pydantic aliases, so reflection missed them and they lost dialect-specific rendering. Signed-off-by: mday-io --- sqlmesh/core/dialect.py | 55 +++++++++++++ tests/core/test_dialect.py | 161 +++++++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+) diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py index ecd43a566f..d9c2bea938 100644 --- a/sqlmesh/core/dialect.py +++ b/sqlmesh/core/dialect.py @@ -804,9 +804,44 @@ def _meta_render_policy() -> t.Dict[str, bool]: for source in sources: for name, field in source.model_fields.items(): policy.setdefault((field.alias or name).lower(), _holds_expression(field.annotation)) + + # `ModelMeta._pre_root_validator` (sqlmesh/core/model/meta.py) renames these two + # user-facing property names to their target field before Pydantic validation, so + # they never surface as a `Field(alias=...)` for the reflection above to find. Give + # each the render policy of the field it is renamed to. + pre_validator_aliases = { + "grain": "grains", + "table_properties": "physical_properties", + } + for alias, target in pre_validator_aliases.items(): + if target in policy: + policy[alias] = policy[target] + return policy +@functools.lru_cache(maxsize=None) +def _dialect_renders_array_as_brackets(dialect_name: t.Optional[str]) -> bool: + """Whether `dialect_name`'s own generator spells an array literal as `[a, b]`. + + Checked by actually rendering a sample `exp.Array` with that dialect, rather than + inspecting `Dialect.ARRAY_SIZE_NAME` or similar generator flags, because the + generator is the single source of truth for what a dialect's array syntax looks + like and there is no single shared flag for it across dialects. This also covers + dialects (tsql, sqlite, tableau, exasol, fabric) that reuse `[`/`]` for identifier + quoting and therefore render arrays as `ARRAY(...)` instead: rewriting their + `tags`/`ignored_rules` value to `[a, b]` would not be an array literal in their + grammar at all, so it silently reparses as one bracket-quoted identifier and + corrupts the value. An unrecognized dialect name renders with the generic + generator, which itself does not use brackets, so it falls back to `False`. + """ + try: + sample = exp.Array(expressions=[exp.Literal.string("x")]) + return sample.sql(dialect=dialect_name).startswith("[") + except Exception: + return False + + def _props_sql(self: Generator, expressions: t.List[exp.Expr]) -> str: props = [] size = len(expressions) @@ -855,6 +890,26 @@ def render_with_model_dialect(node: exp.Expr, **overrides: t.Any) -> str: and _meta_render_policy().get(prop.name.lower()) ): value_sql = render_with_model_dialect(value) + elif ( + meta_dialect + and isinstance(value, exp.Array) + and _dialect_renders_array_as_brackets(meta_dialect) + ): + # Dialect-agnostic properties (e.g. `tags`, `ignored_rules`) that hold a + # list still go through the base (dialect=None) generator, which renders + # an `exp.Array` as `ARRAY(...)`. On BigQuery `ARRAY(` is parsed as a + # subquery constructor, so a multi-element `ARRAY('a', 'b')` fails to + # reparse ("Required keyword: 'value' missing for Property"). Render it + # as a bracketed list literal instead -- but only for dialects that + # actually spell arrays that way; dialects that reuse `[`/`]` for + # identifier quoting (tsql, sqlite, ...) keep the generic `ARRAY(...)` + # form, which they parse back correctly. The elements themselves stay on + # the dialect-agnostic path (`self.expressions`, not + # `render_with_model_dialect`): these are SQLMesh's own scalar values + # (tag/rule name strings), not user warehouse SQL, so they must not be + # transpiled with the model dialect (e.g. tsql boolean literals turning + # into `(1 = 1)`). + value_sql = f"[{self.expressions(value, flat=True)}]" else: value_sql = self.sql(prop, "value") diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index c57ed7e8c4..23c903a66d 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -617,6 +617,167 @@ def test_format_model_expressions_macro_property_comments_preserved_with_dialect assert formatted == twice +@pytest.mark.parametrize("dialect", ["bigquery", "duckdb", "snowflake"]) +@pytest.mark.parametrize("prop_name", ["tags", "ignored_rules"]) +def test_format_model_expressions_list_property_array_literal(dialect: str, prop_name: str): + """Dialect-agnostic header properties that hold a list (`tags`, `ignored_rules`) must + render as a bracketed list literal (`[a, b]`) on dialects that spell arrays that way, + not the base generator's `ARRAY(a, b)`. + + On BigQuery, `ARRAY(` is parsed as a subquery constructor, so a multi-element + `ARRAY('C1', 'c2')` fails to reparse with `Required keyword: 'value' missing for + Property`. This previously affected any dialect using this generator, since these + properties render generically (the `dialect=None` path) regardless of the model's + own dialect. Only bigquery/duckdb/snowflake-like dialects are covered here; + dialects whose own array syntax is not brackets (postgres' `ARRAY[...]`, + databricks' `ARRAY(...)`) or that reuse `[`/`]` for identifier quoting (tsql, + sqlite, ...) are covered by + `test_format_model_expressions_list_property_dialects_without_bracket_arrays`. + """ + source = f"""MODEL ( + name a.b, + dialect {dialect}, + {prop_name} ['C1', 'c2'] +); +SELECT 1 AS x""" + + formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect) + + assert f"{prop_name} ['C1', 'c2']" in formatted + + # Reparses cleanly with the model's own dialect. + reparsed = parse(formatted, default_dialect=dialect) + + # Idempotent: formatting an already-formatted model is a no-op. + twice = format_model_expressions(reparsed, dialect=dialect) + assert formatted == twice + + model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect) + if prop_name == "tags": + assert model.tags == ["C1", "c2"] + else: + assert model.ignored_rules == {"c1", "c2"} + + +def test_format_model_expressions_array_property_no_dialect_unchanged(): + """Regression guard: with no model dialect, list-valued header properties must keep + rendering through the base generator (`ARRAY(...)`), exactly as pinned by + `test_format_model_expressions`. The `[...]` rewrite only applies once a model + dialect is present (gated on `meta_dialect`).""" + formatted = format_model_expressions( + parse("MODEL (name a.b, tags ['C1', 'c2']); SELECT 1 AS x") + ) + + assert "tags ARRAY('C1', 'c2')" in formatted + + +@pytest.mark.parametrize("dialect", ["tsql", "sqlite", "postgres", "databricks"]) +@pytest.mark.parametrize("prop_name", ["tags", "ignored_rules"]) +def test_format_model_expressions_list_property_dialects_without_bracket_arrays( + dialect: str, prop_name: str +): + """Regression: dialects whose own generator does not spell an `exp.Array` as + `[a, b]` must NOT get the bracket-list rewrite from + `test_format_model_expressions_list_property_array_literal`, and must keep the + generic `ARRAY(...)` form. + + This matters most for tsql and sqlite (also true of tableau, exasol, fabric), which + reuse `[`/`]` for identifier quoting: `['a', 'b']` is not an array literal in their + grammar at all, so rewriting `tags` or `ignored_rules` to that form reparses as a + single bracket-quoted identifier, silently collapsing two values into one and + corrupting the tag/rule names -- even though this exact source formatted correctly + on `main` before bracket rendering was introduced. postgres (`ARRAY[...]`) and + databricks (`ARRAY(...)`) are not corrupted by the bracket form, but should still + keep rendering with their own generator's spelling rather than a generic bracket + literal that is not how either dialect writes arrays. + """ + source = f"""MODEL ( + name a.b, + dialect {dialect}, + {prop_name} ARRAY('C1', 'c2') +); +SELECT 1 AS x""" + + formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect) + + prop_line = formatted.split(f"{prop_name} ")[1].split("\n")[0] + assert prop_line.startswith("ARRAY") + assert "[" not in prop_line + + reparsed = parse(formatted, default_dialect=dialect) + twice = format_model_expressions(reparsed, dialect=dialect) + assert formatted == twice + + model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect) + if prop_name == "tags": + assert model.tags == ["C1", "c2"] + else: + assert model.ignored_rules == {"c1", "c2"} + + +@pytest.mark.parametrize("dialect", ["bigquery", "duckdb", "snowflake", "postgres"]) +def test_format_model_expressions_grain_alias_render_policy(dialect: str): + """`grain` is renamed to `grains` in `ModelMeta._pre_root_validator`, not via a + Pydantic alias, so `_meta_render_policy` must special-case it to inherit `grains`' + render policy (warehouse SQL). Otherwise a multi-column `grain [id, id2]` falls back + to the generic, dialect-agnostic path and (via the base generator) becomes + `ARRAY(id, id2)`, which fails to reparse on BigQuery. + """ + source = f"""MODEL ( + name a.b, + dialect {dialect}, + grain [id, id2] +); +SELECT 1 AS x, 2 AS id, 3 AS id2""" + + formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect) + + # Rendered with the model's own dialect (e.g. postgres' native `ARRAY[...]`), never + # the base generator's `ARRAY(id, id2)`, which fails to reparse on BigQuery. + assert "ARRAY(id, id2)" not in formatted + + twice = format_model_expressions(parse(formatted, default_dialect=dialect), dialect=dialect) + assert formatted == twice + + # `grain [id, id2]` parses to a single composite grain wrapping both columns + # (independent of this fix); what matters here is that it survives a dialect- + # specific round trip rather than being flattened to the generic `ARRAY(...)`. + model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect) + assert len(model.grains) == 1 + assert {c.name for c in model.grains[0].find_all(exp.Column)} == {"id", "id2"} + + +def test_format_model_expressions_table_properties_alias_render_policy(): + """`table_properties` is the deprecated alias for `physical_properties`, renamed in + `ModelMeta._pre_root_validator`, not via a Pydantic alias. It must inherit + `physical_properties`' render policy (warehouse SQL) so dialect-specific values + inside it, such as tsql's `DATETIME2`, are not flattened to the generic generator's + `TIMESTAMP` spelling. + """ + formatted = format_model_expressions( + parse( + """ + MODEL ( + name a.b, + dialect tsql, + table_properties ( + x = CAST('2024-01-01' AS DATETIME2) + ) + ); + + SELECT 1 AS x + """, + default_dialect="tsql", + ), + dialect="tsql", + ) + + assert "x = '2024-01-01'::DATETIME2" in formatted + + twice = format_model_expressions(parse(formatted, default_dialect="tsql"), dialect="tsql") + assert formatted == twice + + def test_format_model_expressions_normalize_functions(): """Regression: formatter function-name casing behavior. From f7de34b3af1b20e8bd6ba53e9d3547a06e8072f2 Mon Sep 17 00:00:00 2001 From: mday-io Date: Thu, 24 Sep 2026 16:36:38 +0000 Subject: [PATCH 7/8] test(format): cover `sqlmesh format` on BigQuery header lists Add a Context-level test that formats a BigQuery model with `tags`, `ignored_rules`, `grain` and `partitioned_by`, then reloads it. Trim the header-list unit tests and drop the postgres/databricks cases, which passed with or without the fix. Signed-off-by: mday-io --- tests/core/test_dialect.py | 79 +++++++------------------------------- tests/core/test_format.py | 34 ++++++++++++++++ 2 files changed, 47 insertions(+), 66 deletions(-) diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index 23c903a66d..d2c4358cb0 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -620,20 +620,8 @@ def test_format_model_expressions_macro_property_comments_preserved_with_dialect @pytest.mark.parametrize("dialect", ["bigquery", "duckdb", "snowflake"]) @pytest.mark.parametrize("prop_name", ["tags", "ignored_rules"]) def test_format_model_expressions_list_property_array_literal(dialect: str, prop_name: str): - """Dialect-agnostic header properties that hold a list (`tags`, `ignored_rules`) must - render as a bracketed list literal (`[a, b]`) on dialects that spell arrays that way, - not the base generator's `ARRAY(a, b)`. - - On BigQuery, `ARRAY(` is parsed as a subquery constructor, so a multi-element - `ARRAY('C1', 'c2')` fails to reparse with `Required keyword: 'value' missing for - Property`. This previously affected any dialect using this generator, since these - properties render generically (the `dialect=None` path) regardless of the model's - own dialect. Only bigquery/duckdb/snowflake-like dialects are covered here; - dialects whose own array syntax is not brackets (postgres' `ARRAY[...]`, - databricks' `ARRAY(...)`) or that reuse `[`/`]` for identifier quoting (tsql, - sqlite, ...) are covered by - `test_format_model_expressions_list_property_dialects_without_bracket_arrays`. - """ + """List-valued header properties render as `[a, b]`, not `ARRAY(a, b)`, which + BigQuery parses as a subquery and fails to load.""" source = f"""MODEL ( name a.b, dialect {dialect}, @@ -642,14 +630,9 @@ def test_format_model_expressions_list_property_array_literal(dialect: str, prop SELECT 1 AS x""" formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect) - assert f"{prop_name} ['C1', 'c2']" in formatted - # Reparses cleanly with the model's own dialect. - reparsed = parse(formatted, default_dialect=dialect) - - # Idempotent: formatting an already-formatted model is a no-op. - twice = format_model_expressions(reparsed, dialect=dialect) + twice = format_model_expressions(parse(formatted, default_dialect=dialect), dialect=dialect) assert formatted == twice model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect) @@ -660,10 +643,6 @@ def test_format_model_expressions_list_property_array_literal(dialect: str, prop def test_format_model_expressions_array_property_no_dialect_unchanged(): - """Regression guard: with no model dialect, list-valued header properties must keep - rendering through the base generator (`ARRAY(...)`), exactly as pinned by - `test_format_model_expressions`. The `[...]` rewrite only applies once a model - dialect is present (gated on `meta_dialect`).""" formatted = format_model_expressions( parse("MODEL (name a.b, tags ['C1', 'c2']); SELECT 1 AS x") ) @@ -671,26 +650,13 @@ def test_format_model_expressions_array_property_no_dialect_unchanged(): assert "tags ARRAY('C1', 'c2')" in formatted -@pytest.mark.parametrize("dialect", ["tsql", "sqlite", "postgres", "databricks"]) +@pytest.mark.parametrize("dialect", ["tsql", "sqlite"]) @pytest.mark.parametrize("prop_name", ["tags", "ignored_rules"]) -def test_format_model_expressions_list_property_dialects_without_bracket_arrays( +def test_format_model_expressions_list_property_bracket_identifier_dialects( dialect: str, prop_name: str ): - """Regression: dialects whose own generator does not spell an `exp.Array` as - `[a, b]` must NOT get the bracket-list rewrite from - `test_format_model_expressions_list_property_array_literal`, and must keep the - generic `ARRAY(...)` form. - - This matters most for tsql and sqlite (also true of tableau, exasol, fabric), which - reuse `[`/`]` for identifier quoting: `['a', 'b']` is not an array literal in their - grammar at all, so rewriting `tags` or `ignored_rules` to that form reparses as a - single bracket-quoted identifier, silently collapsing two values into one and - corrupting the tag/rule names -- even though this exact source formatted correctly - on `main` before bracket rendering was introduced. postgres (`ARRAY[...]`) and - databricks (`ARRAY(...)`) are not corrupted by the bracket form, but should still - keep rendering with their own generator's spelling rather than a generic bracket - literal that is not how either dialect writes arrays. - """ + """These dialects quote identifiers with `[...]`, so a bracketed list would reload + as a single identifier. List properties must keep the `ARRAY(...)` form.""" source = f"""MODEL ( name a.b, dialect {dialect}, @@ -699,13 +665,9 @@ def test_format_model_expressions_list_property_dialects_without_bracket_arrays( SELECT 1 AS x""" formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect) + assert f"{prop_name} ARRAY('C1', 'c2')" in formatted - prop_line = formatted.split(f"{prop_name} ")[1].split("\n")[0] - assert prop_line.startswith("ARRAY") - assert "[" not in prop_line - - reparsed = parse(formatted, default_dialect=dialect) - twice = format_model_expressions(reparsed, dialect=dialect) + twice = format_model_expressions(parse(formatted, default_dialect=dialect), dialect=dialect) assert formatted == twice model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect) @@ -717,12 +679,8 @@ def test_format_model_expressions_list_property_dialects_without_bracket_arrays( @pytest.mark.parametrize("dialect", ["bigquery", "duckdb", "snowflake", "postgres"]) def test_format_model_expressions_grain_alias_render_policy(dialect: str): - """`grain` is renamed to `grains` in `ModelMeta._pre_root_validator`, not via a - Pydantic alias, so `_meta_render_policy` must special-case it to inherit `grains`' - render policy (warehouse SQL). Otherwise a multi-column `grain [id, id2]` falls back - to the generic, dialect-agnostic path and (via the base generator) becomes - `ARRAY(id, id2)`, which fails to reparse on BigQuery. - """ + """`grain` is renamed to `grains` before validation, so it must share the + `grains` render policy instead of falling back to `ARRAY(...)`.""" source = f"""MODEL ( name a.b, dialect {dialect}, @@ -731,29 +689,18 @@ def test_format_model_expressions_grain_alias_render_policy(dialect: str): SELECT 1 AS x, 2 AS id, 3 AS id2""" formatted = format_model_expressions(parse(source, default_dialect=dialect), dialect=dialect) - - # Rendered with the model's own dialect (e.g. postgres' native `ARRAY[...]`), never - # the base generator's `ARRAY(id, id2)`, which fails to reparse on BigQuery. assert "ARRAY(id, id2)" not in formatted twice = format_model_expressions(parse(formatted, default_dialect=dialect), dialect=dialect) assert formatted == twice - # `grain [id, id2]` parses to a single composite grain wrapping both columns - # (independent of this fix); what matters here is that it survives a dialect- - # specific round trip rather than being flattened to the generic `ARRAY(...)`. model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect) - assert len(model.grains) == 1 assert {c.name for c in model.grains[0].find_all(exp.Column)} == {"id", "id2"} def test_format_model_expressions_table_properties_alias_render_policy(): - """`table_properties` is the deprecated alias for `physical_properties`, renamed in - `ModelMeta._pre_root_validator`, not via a Pydantic alias. It must inherit - `physical_properties`' render policy (warehouse SQL) so dialect-specific values - inside it, such as tsql's `DATETIME2`, are not flattened to the generic generator's - `TIMESTAMP` spelling. - """ + """`table_properties` is renamed to `physical_properties` before validation, so it + must keep dialect-specific types such as tsql's `DATETIME2`.""" formatted = format_model_expressions( parse( """ diff --git a/tests/core/test_format.py b/tests/core/test_format.py index 5a44e1b381..481c5c65b5 100644 --- a/tests/core/test_format.py +++ b/tests/core/test_format.py @@ -161,3 +161,37 @@ def test_format_without_state_load(tmp_path: pathlib.Path, mocker: MockerFixture context = Context(paths=tmp_path, config=Config(project="local_only"), load_state=False) context.format(check=True) mock.assert_not_called() + + +def test_format_bigquery_header_list_properties(tmp_path: pathlib.Path): + # A BigQuery model must survive `sqlmesh format` and still load: list-valued header + # properties were rewritten to `ARRAY(...)`, which BigQuery parses as a subquery. + model_file = create_temp_file( + tmp_path, + pathlib.Path("models/model.sql"), + """MODEL ( + name test.model, + kind INCREMENTAL_BY_TIME_RANGE (time_column ds), + tags ['C1', 'c2'], + ignored_rules ['noselectstar', 'ambiguousorinvalidcolumn'], + grain [id], + partitioned_by DATE_TRUNC(ds, MONTH) +); +SELECT 1 AS id, CURRENT_DATE() AS ds""", + ) + config = Config(model_defaults=ModelDefaultsConfig(dialect="bigquery")) + + Context(paths=tmp_path, config=config).format() + + formatted = model_file.read_text(encoding="utf-8") + assert "tags ['C1', 'c2']" in formatted + assert "ignored_rules ['noselectstar', 'ambiguousorinvalidcolumn']" in formatted + assert "grain [id]" in formatted + assert "partitioned_by DATE_TRUNC(ds, MONTH)" in formatted + + context = Context(paths=tmp_path, config=config) + assert context.format(check=True) + model = context.get_model("test.model") + assert model.tags == ["C1", "c2"] + assert model.ignored_rules == {"noselectstar", "ambiguousorinvalidcolumn"} + assert [p.sql("bigquery") for p in model.partitioned_by] == ["DATE_TRUNC(`ds`, MONTH)"] From 68da7f440b42bb6dc456ea74f1f2ad4117484133 Mon Sep 17 00:00:00 2001 From: mday-io Date: Fri, 25 Sep 2026 14:49:18 +0000 Subject: [PATCH 8/8] fix(format): keep boolean literals stable in dialect-rendered header values On tsql, a boolean inside a header value rendered with the model dialect, such as `audits (my_audit(flag := true))`, was written as `(1 = 1)` and then wrapped across lines on the next run, so `sqlmesh format --check` flagged an already-formatted model. Keep boolean literals as `TRUE`/`FALSE` there, as before, since the value is transpiled with the model dialect when it is used. Also add a regression test that repeated formatting keeps a model's declared column types (BigQuery `DATETIME`, tsql `DATETIME2`), and merge the `time_column` kind test into the equivalent scalar-sibling test. Signed-off-by: mday-io --- sqlmesh/core/dialect.py | 14 ++++++- tests/core/test_dialect.py | 81 +++++++++++++++----------------------- 2 files changed, 45 insertions(+), 50 deletions(-) diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py index d9c2bea938..bcfb300ba1 100644 --- a/sqlmesh/core/dialect.py +++ b/sqlmesh/core/dialect.py @@ -864,7 +864,19 @@ def render_with_model_dialect(node: exp.Expr, **overrides: t.Any) -> str: "comments": self.comments, } opts.update(overrides) - return node.sql(**opts) + + # Keep boolean literals anywhere in the value (audit args, physical_properties, + # merge_filter, ...) as `TRUE`/`FALSE`: tsql would otherwise emit `(1 = 1)`, + # which reformats differently on the next pass. The value is transpiled with + # the model dialect anyway when it is used, e.g. in the rendered audit query. + def keep_boolean_literal(n: exp.Expr) -> exp.Expr: + if not isinstance(n, exp.Boolean): + return n + literal = exp.var("TRUE" if n.this else "FALSE") + literal.comments = n.comments + return literal + + return node.transform(keep_boolean_literal).sql(**opts) if isinstance(prop, MacroFunc): # A macro in property position wraps user-authored arguments, so it carries diff --git a/tests/core/test_dialect.py b/tests/core/test_dialect.py index d2c4358cb0..dc55571f21 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -422,8 +422,8 @@ def test_format_model_expressions_meta_render_policy(dialect: str, audit_type: s @pytest.mark.parametrize( "header", [ - "columns (ts DATETIME2(6))", "audits (my_audit(t := CAST('2024-01-01' AS DATETIME2)))", + "audits (my_audit(flag := true))", "kind SCD_TYPE_2_BY_COLUMN(unique_key id, columns (a, b), time_data_type DATETIME2(6))", "physical_properties (labels = (('env', 'prod')))", "allow_partials true, description 'my description'", @@ -447,6 +447,28 @@ def test_format_model_expressions_is_idempotent(header: str): assert once == twice +@pytest.mark.parametrize( + "dialect,column_type", + [("bigquery", "DATETIME"), ("tsql", "DATETIME2(6)")], +) +def test_format_model_expressions_preserves_column_types(dialect: str, column_type: str): + """Repeated `sqlmesh format` runs must not change a model's declared column types. + + Rendering `columns` with the generic generator rewrote them: BigQuery `DATETIME` + became `TIMESTAMP` and then `TIMESTAMPTZ`, tsql `DATETIME2` became `TIMESTAMP` and + then `VARBINARY`. + """ + expected = exp.DataType.build(column_type, dialect=dialect) + formatted = f"MODEL (name a.b, dialect {dialect}, columns (ts {column_type}));\nSELECT 1 AS ts" + + for _ in range(2): + formatted = format_model_expressions( + parse(formatted, default_dialect=dialect), dialect=dialect + ) + model = load_sql_based_model(parse(formatted, default_dialect=dialect), dialect=dialect) + assert model.columns_to_types == {"ts": expected} + + def test_format_audit_expressions_meta_render_policy(): """AUDIT headers have their own meta model, and get the same split: `blocking` is SQLMesh's own boolean and must not become tsql's `(1 = 0)`, while `defaults` holds @@ -473,60 +495,21 @@ def test_format_audit_expressions_meta_render_policy(): assert "cutoff := '2024-01-01'::DATETIME2" in formatted -def test_format_model_expressions_time_column_dialect(): - """`time_column` is a nested Pydantic model (`TimeColumn`) wrapping an expression, not +def test_format_model_expressions_kind_time_column_dialect(): + """Expression-bearing properties nested inside `kind` render with the model dialect, + while their scalar siblings stay dialect-agnostic. + + `time_column` is a nested Pydantic model (`TimeColumn`) wrapping an expression, not an `exp.Expr` annotation itself, so the render-policy reflection must recurse into nested Pydantic models to classify it as warehouse SQL. Otherwise it falls back to generic rendering and loses dialect-specific identifier quoting: tsql's `[end]` becomes ANSI `"end"`, even though the same identifier in the query body is correctly kept as `[end]`. - """ - formatted = format_model_expressions( - parse( - """ - MODEL ( - name a.b, - dialect tsql, - kind INCREMENTAL_BY_TIME_RANGE ( - time_column [end] - ) - ); - - SELECT 1 AS x, [end] FROM t - """, - default_dialect="tsql", - ), - dialect="tsql", - ) - - assert ( - formatted - == """MODEL ( - name a.b, - dialect tsql, - kind INCREMENTAL_BY_TIME_RANGE ( - time_column [end] - ) -); - -SELECT - 1 AS x, - [end] -FROM t""" - ) - - -def test_format_model_expressions_kind_scalar_sibling_dialect(): - """A scalar sibling property of an expression-bearing property inside `kind` (e.g. - `forward_only` next to `time_column`) must stay dialect-agnostic even though the - render policy correctly marks `kind` as containing an expression-holding field - somewhere in the `ModelKind` union. - Regression: recursing into nested Pydantic models to fix `time_column` (see - `test_format_model_expressions_time_column_dialect`) made `_holds_expression` also - match on `kind` itself, since *some* member of the `ModelKind` union - (`IncrementalByTimeRangeKind.time_column`) holds an expression. That routed the - entire `kind (...)` subtree through a dialect-specific generator, so tsql's + Regression: recursing into nested Pydantic models to fix `time_column` made + `_holds_expression` also match on `kind` itself, since *some* member of the + `ModelKind` union (`IncrementalByTimeRangeKind.time_column`) holds an expression. + That routed the entire `kind (...)` subtree through a dialect-specific generator, so tsql's boolean-literal preprocessing rewrote `forward_only TRUE` into `forward_only (1 = 1)`. That reparses without error, but `str_to_bool` on `Paren(EQ(1, 1)).name` (`""`) evaluates to `False`, so the value silently flips on reload.