diff --git a/sqlmesh/core/dialect.py b/sqlmesh/core/dialect.py index e4ab522198..ecd43a566f 100644 --- a/sqlmesh/core/dialect.py +++ b/sqlmesh/core/dialect.py @@ -734,15 +734,131 @@ def parse(self: Parser) -> t.Optional[exp.Expr]: } +_SQLMESH_META_DIALECT = "sqlmesh_meta_dialect" + + +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], 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). + + 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 + 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) +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) 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. 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: - sql = self.indent(f"{prop.name} {self.sql(prop, 'value')}") + value = prop.args.get("value") + + if ( + meta_dialect + and isinstance(value, exp.Expr) + and _meta_render_policy().get(prop.name.lower()) + ): + value_sql = render_with_model_dialect(value) + else: + value_sql = self.sql(prop, "value") + + sql = self.indent(f"{prop.name} {value_sql}") if i < size - 1: sql += "," @@ -853,11 +969,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 +1027,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..c57ed7e8c4 100644 --- a/tests/core/test_dialect.py +++ b/tests/core/test_dialect.py @@ -342,6 +342,281 @@ 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""" + ) + + +@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'", + "@my_prop(cutoff := CAST('2024-01-01' AS DATETIME2))", + ], +) +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 + 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_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_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. + + 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.