Repository: TobikoData/sqlmesh (python)

Issue:
Inconsistencies with the `@this_model` macro
The `@this_model` macro should resolve to a physical layer reference according to [its documentation](https://sqlmesh.readthedocs.io/en/stable/concepts/macros/macro_variables/#runtime-variables):
```
@this_model - A string value containing the name of the physical table the model view selects from. Typically used to create generic audits.
```

Currently, when used in a model, `@this_model` [returns a string](https://github.com/TobikoData/sqlmesh/blob/main/sqlmesh/core/renderer.py#L108). However in audits, it returns a [select expression](https://github.com/TobikoData/sqlmesh/blob/main/sqlmesh/core/model/definition.py#L484).

The problem with returning a string is that it limits the use of `@this_model` in pre/post statements. The fully rendered string literal gets re-parsed as an identfier, leading to double quoting.

For example, in a post statement:
```
create table if not exists foo as select * from @this_model;
```

A plan will fail because SQLMesh tries to execute invalid SQL like:
```
CREATE TABLE IF NOT EXISTS "foo" AS SELECT * FROM """db"".""erin_staging"".""staging__test5__210450832""" AS """db"".""erin_staging"".""staging__test5__210450832"""
```

It only works correctly with the Jinja escape hatch:
```
JINJA_STATEMENT_BEGIN;
create table if not exists foo as select * from {{ this_model }};
JINJA_END;
```

Since SQLMesh macros should be first class and we should only have to drop to Jinja to work around current SQLGlot limitations, would it be worth:
 - Making `@this_model` always return an expression that will evaluate to the physical layer object. This will allow it to be used in the native SQLMesh dialect
 - Rendering any Jinja context variables of type `exp.Expression` to strings via `value.sql(dialect=...)`  before passing them to Jinja

This would allow SQLMesh native to work with SQLGLot AST nodes as it is designed and Jinja to work with strings, as it is designed

Pull request:
Fix!: store a Table in @this_model var instead of its str representation
Fixes #3444

Patch:
diff --git a/sqlmesh/core/renderer.py b/sqlmesh/core/renderer.py
index 0e5eb3c6..88238e63 100644
--- a/sqlmesh/core/renderer.py
+++ b/sqlmesh/core/renderer.py
@@ -103,9 +103,11 @@ class BaseExpressionRenderer:
         if should_cache and self._cache:
             return self._cache
 
-        if self._model_fqn and "this_model" not in kwargs:
+        this_model = kwargs.pop("this_model", None)
+
+        if not this_model and self._model_fqn:
             this_snapshot = (snapshots or {}).get(self._model_fqn)
-            kwargs["this_model"] = self._resolve_table(
+            this_model = self._resolve_table(
                 self._model_fqn,
                 snapshots={self._model_fqn: this_snapshot} if this_snapshot else None,
                 deployability_index=deployability_index,
@@ -123,20 +125,27 @@ class BaseExpressionRenderer:
         }
 
         variables = kwargs.pop("variables", {})
-        jinja_env = self._jinja_macro_registry.build_environment(
+        jinja_env_kwargs = {
             **{**render_kwargs, **prepare_env(self._python_env), **variables},
-            snapshots=(snapshots or {}),
-            table_mapping=table_mapping,
-            deployability_index=deployability_index,
-            default_catalog=self._default_catalog,
-            runtime_stage=runtime_stage.value,
-            resolve_table=lambda table: self._resolve_table(
+            "snapshots": snapshots or {},
+            "table_mapping": table_mapping,
+            "deployability_index": deployability_index,
+            "default_catalog": self._default_catalog,
+            "runtime_stage": runtime_stage.value,
+            "resolve_table": lambda table: self._resolve_table(
                 table,
                 snapshots=snapshots,
                 table_mapping=table_mapping,
                 deployability_index=deployability_index,
-            ),
-        )
+            ).sql(dialect=self._dialect, identify=True, comments=False),
+        }
+        if this_model:
+            render_kwargs["this_model"] = this_model
+            jinja_env_kwargs["this_model"] = this_model.sql(
+                dialect=self._dialect, identify=True, comments=False
+            )
+
+        jinja_env = self._jinja_macro_registry.build_environment(**jinja_env_kwargs)
 
         if isinstance(self._expression, d.Jinja):
             try:
@@ -230,7 +239,7 @@ class BaseExpressionRenderer:
         snapshots: t.Optional[t.Dict[str, Snapshot]] = None,
         table_mapping: t.Optional[t.Dict[str, str]] = None,
         deployability_index: t.Optional[DeployabilityIndex] = None,
-    ) -> str:
+    ) -> exp.Table:
         return exp.replace_tables(
             exp.maybe_parse(table_name, into=exp.Table, dialect=self._dialect),
             {
@@ -239,7 +248,7 @@ class BaseExpressionRenderer:
             },
             dialect=self._dialect,
             copy=False,
-        ).sql(dialect=self._dialect, identify=True, comments=False)
+        )
 
     def _resolve_tables(
         self,


Test patch:
diff --git a/tests/core/test_model.py b/tests/core/test_model.py
index f48b108e..6236bfc9 100644
--- a/tests/core/test_model.py
+++ b/tests/core/test_model.py
@@ -690,6 +690,20 @@ 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(