Repository: TobikoData/sqlmesh
Pull request #3494 (2024-12-10 11:39:53)

Fix: quote the table produced by _resolve_table
During the recent refactor, where `this_model` changed from a `str` into a `Table` instance, I missed the fact that we were previously [quoting](https://github.com/TobikoData/sqlmesh/commit/b00d7d40b42ff759243b161567a81547c1002486#diff-fddedea4eda285dbd51b83821f3cd54dc42d3946c0056760acdc29efc3e565afL242) it, so I didn't match that behavior.

We discovered that this led to an issue where the resolved table for `this_model` would be normalized twice, if e.g. it was interpolated in a string, and hence generated as a table with unquoted identifiers.

This PR fixes the above issue by having `_resolve_table` quote the table after resolving it, to match the semantics of `_resolve_tables`.


Patch:
diff --git a/sqlmesh/core/renderer.py b/sqlmesh/core/renderer.py
index 88238e63..5f1b651e 100644
--- a/sqlmesh/core/renderer.py
+++ b/sqlmesh/core/renderer.py
@@ -240,7 +240,7 @@ class BaseExpressionRenderer:
         table_mapping: t.Optional[t.Dict[str, str]] = None,
         deployability_index: t.Optional[DeployabilityIndex] = None,
     ) -> exp.Table:
-        return exp.replace_tables(
+        table = exp.replace_tables(
             exp.maybe_parse(table_name, into=exp.Table, dialect=self._dialect),
             {
                 **self._to_table_mapping((snapshots or {}).values(), deployability_index),
@@ -249,6 +249,11 @@ class BaseExpressionRenderer:
             dialect=self._dialect,
             copy=False,
         )
+        # We quote the table here to mimic the behavior of _resolve_tables, otherwise we may end
+        # up normalizing twice, because _to_table_mapping returns the mapped names unquoted.
+        return (
+            d.quote_identifiers(table, dialect=self._dialect) if self._quote_identifiers else table
+        )
 
     def _resolve_tables(
         self,


Test patch:
diff --git a/tests/core/test_model.py b/tests/core/test_model.py
index 00e1142c..335d933a 100644
--- a/tests/core/test_model.py
+++ b/tests/core/test_model.py
@@ -691,20 +691,6 @@ def test_model_pre_post_statements():
     assert model.render_post_statements() == expected_post
     assert "exp" in model.python_env
 
-    expressions = d.parse(
-        """
-        MODEL (name db.table);
-
-        SELECT 1 AS col;
-
-        CREATE TABLE db.other AS SELECT * FROM @this_model;
-        """
-    )
-    model = load_sql_based_model(expressions)
-
-    expected_post = d.parse('CREATE TABLE "db"."other" AS SELECT * FROM "db"."table" AS "table";')
-    assert model.render_post_statements() == expected_post
-
 
 def test_seed_hydration():
     expressions = d.parse(
@@ -4954,6 +4940,57 @@ def test_this_model() -> None:
         == f"SELECT '`sqlmesh__project-1`.`project_1__table__{snapshot.version}`' AS `x`"
     )
 
+    @macro()
+    def this_model_resolves_to_quoted_table(evaluator):
+        this_model = evaluator.locals.get("this_model")
+        if not this_model:
+            return True
+
+        this_snapshot = evaluator.get_snapshot("db.table")
+        if this_snapshot and this_snapshot.version:
+            # If the table wasn't quoted, we'd break the `sqlmesh_DB` reference by
+            # normalizing it twice
+            expected_name = f'"sqlmesh__DB"."DB__TABLE__{this_snapshot.version}"'
+        else:
+            expected_name = '"DB"."TABLE"'
+
+        return not this_model or (
+            isinstance(this_model, exp.Table)
+            and this_model.sql(dialect=evaluator.dialect, comments=False) == expected_name
+        )
+
+    expressions = d.parse(
+        """
+        MODEL (name db.table, dialect snowflake);
+
+        SELECT
+          1 AS col,
+          @this_model_resolves_to_quoted_table() AS this_model_resolves_to_quoted_table;
+
+        CREATE TABLE db.other AS SELECT * FROM @this_model AS x;
+        """
+    )
+    model = load_sql_based_model(expressions)
+
+    expected_post = d.parse('CREATE TABLE "DB"."OTHER" AS SELECT * FROM "DB"."TABLE" AS "X";')
+    assert model.render_post_statements() == expected_post
+
+    snapshot = Snapshot.from_node(model, nodes={})
+    assert (
+        model.render_query_or_raise(snapshots={snapshot.name: snapshot}, start="2020-01-01").sql(
+            dialect="snowflake"
+        )
+        == 'SELECT 1 AS "COL", TRUE AS "THIS_MODEL_RESOLVES_TO_QUOTED_TABLE"'
+    )
+
+    snapshot.categorize_as(SnapshotChangeCategory.BREAKING)
+    assert (
+        model.render_query_or_raise(snapshots={snapshot.name: snapshot}, start="2021-01-01").sql(
+            dialect="snowflake"
+        )
+        == 'SELECT 1 AS "COL", TRUE AS "THIS_MODEL_RESOLVES_TO_QUOTED_TABLE"'
+    )
+
 
 def test_macros_in_model_statement(sushi_context, assert_exp_eq):
     @macro()