diff --git a/docs/concepts/macros/sqlmesh_macros.md b/docs/concepts/macros/sqlmesh_macros.md index c7d967b12c..5459d79ca8 100644 --- a/docs/concepts/macros/sqlmesh_macros.md +++ b/docs/concepts/macros/sqlmesh_macros.md @@ -1608,7 +1608,7 @@ def add_args( return argument_1 + argument_2 + argument_3 ``` -An `@add_args` call providing values for all arguments accepts positional arguments like this: `@add_args(5, 6, 7)` (which returns 5 + 6 + 7 = `18`). A call omitting and using the default value for the the final `argument_3` can also use positional arguments: `@add_args(5, 6)` (which returns 5 + 6 + 3 = `14`). +An `@add_args` call providing values for all arguments accepts positional arguments like this: `@add_args(5, 6, 7)` (which returns 5 + 6 + 7 = `18`). A call omitting and using the default value for the final `argument_3` can also use positional arguments: `@add_args(5, 6)` (which returns 5 + 6 + 3 = `14`). However, skipping an argument requires specifying the names of subsequent arguments (i.e., using "keyword arguments"). For example, skipping the second argument above by just omitting it - `@add_args(5, , 7)` - results in an error. diff --git a/docs/concepts/models/model_kinds.md b/docs/concepts/models/model_kinds.md index d01cc738a6..cde104790a 100644 --- a/docs/concepts/models/model_kinds.md +++ b/docs/concepts/models/model_kinds.md @@ -583,7 +583,7 @@ MODEL ( ### When Matched Expression -The logic to use when updating columns when a match occurs (the source and target match on the given keys) by default updates all the columns. This can be overriden with custom logic like below: +The logic to use when updating columns when a match occurs (the source and target match on the given keys) by default updates all the columns. This can be overridden with custom logic like below: ```sql linenums="1" hl_lines="5" MODEL ( @@ -1437,7 +1437,7 @@ GROUP BY SCD Type 2 models are designed by default to protect the data that has been captured because it is not possible to recreate the history once it has been lost. However, there are cases where you may want to clear the history and start fresh. -For this use use case you will want to start by setting `disable_restatement` to `false` in the model definition. +For this use case you will want to start by setting `disable_restatement` to `false` in the model definition. ```sql linenums="1" hl_lines="5" MODEL ( diff --git a/docs/guides/ui.md b/docs/guides/ui.md index 29bb204988..fa93b2448c 100644 --- a/docs/guides/ui.md +++ b/docs/guides/ui.md @@ -225,7 +225,7 @@ You may include all a project's models by clicking `All` in the Show drop-down o ![Lineage module - all models](./ui/ui-guide_lineage-all.png){ loading=lazy } -Click `Connected` in the Show drop-down menu to highlight edges between upstream parents and downstream children in blue. This may be helpful when when a project contains many models: +Click `Connected` in the Show drop-down menu to highlight edges between upstream parents and downstream children in blue. This may be helpful when a project contains many models: ![Lineage module - all models, connected edges](./ui/ui-guide_lineage-all-connected.png){ loading=lazy } diff --git a/docs/quickstart/notebook.md b/docs/quickstart/notebook.md index a1dae6b822..0398fc7038 100644 --- a/docs/quickstart/notebook.md +++ b/docs/quickstart/notebook.md @@ -248,7 +248,7 @@ You've now created a new production environment with all of history backfilled. ## 3. Update a model -Now that we have have populated the `prod` environment, let's modify one of the SQL models. +Now that we have populated the `prod` environment, let's modify one of the SQL models. We can modify the incremental SQL model using the `%model` *line* notebook magic (note the single `%`) and the model name: 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.