From ecc4784c8d84b4f27f7710a48b9e95c80c518dcc Mon Sep 17 00:00:00 2001 From: mday-io Date: Thu, 6 Aug 2026 18:29:34 +0000 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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.