pr feedback; add unit tests

Author: themisvaltinos

sqlmesh/core/renderer.py

@@ -5,11 +5,6 @@
 from contextlib import contextmanager
 from functools import partial
 from pathlib import Path
-import re
-from sys import exc_info
-from traceback import walk_tb
-from jinja2 import UndefinedError
-from jinja2.runtime import Macro
 
 from sqlglot import exp, parse
 from sqlglot.errors import SqlglotError
@@ -35,7 +30,7 @@
     SQLMeshError,
     raise_config_error,
 )
-from sqlmesh.utils.jinja import JinjaMacroRegistry
+from sqlmesh.utils.jinja import JinjaMacroRegistry, extract_error_details
 from sqlmesh.utils.metaprogramming import Executable, prepare_env
 
 if t.TYPE_CHECKING:
@@ -252,43 +247,9 @@ def _resolve_table(table: str | exp.Table) -> str:
             except ParsetimeAdapterCallError:
                 raise
             except Exception as ex:
-                error_details: t.List[str] = []
-                if isinstance(ex, UndefinedError):
-                    try:
-                        _, _, exc_traceback = exc_info()
-                        for frame, _ in walk_tb(exc_traceback):
-                            if frame.f_code.co_name == "_invoke":
-                                macro = frame.f_locals.get("self")
-                                if isinstance(macro, Macro):
-                                    arguments = frame.f_locals.get("arguments", [])
-                                    arg_strs = [
-                                        f"'{a}'" if isinstance(a, str) else str(a)
-                                        for a in arguments
-                                    ]
-                                    error_details.append(
-                                        f"\nError when calling jinja macro: {macro.name}({', '.join(arg_strs)})\n"
-                                    )
-                                    for package in self._jinja_macro_registry.packages:
-                                        try:
-                                            if macro_info := self._jinja_macro_registry._get_macro(
-                                                macro.name, package
-                                            ):
-                                                error_details.append(
-                                                    "Macro definition:\n" + macro_info.definition
-                                                )
-                                                break
-                                        except:
-                                            pass
-                                    break
-                    except:
-                        # fall back to the generic error message if frame analysis fails
-                        pass
-
-                    if match := re.search(r"'(\w+)'", str(ex)):
-                        error_details.append(f"\nUndefined macro/variable: '{match.group(1)}'\n")
-
-                error_msg = f"Could not render or parse jinja at '{self._path}'."
-                error_msg += "\n" + "\n".join(error_details) if error_details else f"\n{ex}"
+                error_msg = f"Could not render or parse jinja for '{self._path}'.\n" + (
+                    extract_error_details(ex) or f"{ex}"
+                )
 
                 raise ConfigError(error_msg) from ex
 

sqlmesh/utils/jinja.py

@@ -7,8 +7,11 @@
 import zlib
 from collections import defaultdict
 from enum import Enum
+from sys import exc_info
+from traceback import walk_tb
 
-from jinja2 import Environment, Template, nodes
+from jinja2 import Environment, Template, nodes, UndefinedError
+from jinja2.runtime import Macro
 from sqlglot import Dialect, Expression, Parser, TokenType
 
 from sqlmesh.core import constants as c
@@ -664,3 +667,24 @@ def make_jinja_registry(
     jinja_registry = jinja_registry.trim(jinja_references)
 
     return jinja_registry
+
+
+def extract_error_details(ex: Exception) -> str:
+    """Extracts a readable message from a Jinja2 error, to include missing name and macro."""
+
+    error_details = ""
+    if isinstance(ex, UndefinedError):
+        if match := re.search(r"'(\w+)'", str(ex)):
+            error_details += f"\nUndefined macro/variable: '{match.group(1)}'"
+        try:
+            _, _, exc_traceback = exc_info()
+            for frame, _ in walk_tb(exc_traceback):
+                if frame.f_code.co_name == "_invoke":
+                    macro = frame.f_locals.get("self")
+                    if isinstance(macro, Macro):
+                        error_details += f" in macro: {macro.name}\n"
+                        break
+        except:
+            # to fall back to the generic error message if frame analysis fails
+            pass
+    return error_details

tests/core/test_context.py

@@ -631,6 +631,49 @@ def test_env_and_default_schema_normalization(mocker: MockerFixture):
     assert list(context.fetchdf('select c from "DEFAULT__DEV"."X"')["c"])[0] == 1
 
 
+def test_jinja_macro_undefined_variable_error(tmp_path: pathlib.Path):
+    models_dir = tmp_path / "models"
+    models_dir.mkdir(parents=True)
+    macros_dir = tmp_path / "macros"
+    macros_dir.mkdir(parents=True)
+
+    macro_file = macros_dir / "my_macros.sql"
+    macro_file.write_text("""
+{%- macro generate_select(table_name) -%}
+  {%- if target.name == 'production' -%}
+    {%- set results = run_query('SELECT 1') -%}
+  {%- endif -%}
+  SELECT {{ results.columns[0].values()[0] }} FROM {{ table_name }}
+{%- endmacro -%}
+""")
+
+    model_file = models_dir / "my_model.sql"
+    model_file.write_text("""
+MODEL (
+    name my_schema.my_model,
+    kind FULL
+);
+
+JINJA_QUERY_BEGIN;
+{{ generate_select('users') }}
+JINJA_END;
+""")
+
+    config_file = tmp_path / "config.yaml"
+    config_file.write_text("""
+model_defaults:
+  dialect: duckdb
+""")
+
+    with pytest.raises(ConfigError) as exc_info:
+        Context(paths=str(tmp_path))
+
+    error_message = str(exc_info.value)
+    assert "Failed to load model" in error_message
+    assert "Could not render or parse jinja for" in error_message
+    assert "Undefined macro/variable: 'target' in macro: generate_select" in error_message
+
+
 def test_clear_caches(tmp_path: pathlib.Path):
     models_dir = tmp_path / "models"
 
@@ -2497,7 +2540,7 @@ def test_plan_min_intervals(tmp_path: Path):
       ),
       start '2020-01-01',
       cron '@daily'
-    );                        
+    );
 
     select @start_ds as start_ds, @end_ds as end_ds, @start_dt as start_dt, @end_dt as end_dt;
     """)
@@ -2510,9 +2553,9 @@ def test_plan_min_intervals(tmp_path: Path):
       ),
       start '2020-01-01',
       cron '@weekly'
-    );                        
+    );
 
-    select @start_ds as start_ds, @end_ds as end_ds, @start_dt as start_dt, @end_dt as end_dt;                        
+    select @start_ds as start_ds, @end_ds as end_ds, @start_dt as start_dt, @end_dt as end_dt;
     """)
 
     (tmp_path / "models" / "monthly_model.sql").write_text("""
@@ -2523,9 +2566,9 @@ def test_plan_min_intervals(tmp_path: Path):
       ),
       start '2020-01-01',
       cron '@monthly'
-    );                        
+    );
 
-    select @start_ds as start_ds, @end_ds as end_ds, @start_dt as start_dt, @end_dt as end_dt;                         
+    select @start_ds as start_ds, @end_ds as end_ds, @start_dt as start_dt, @end_dt as end_dt;
     """)
 
     (tmp_path / "models" / "ended_daily_model.sql").write_text("""
@@ -2537,9 +2580,9 @@ def test_plan_min_intervals(tmp_path: Path):
       start '2020-01-01',
       end '2020-01-18',
       cron '@daily'
-    );                        
+    );
 
-    select @start_ds as start_ds, @end_ds as end_ds, @start_dt as start_dt, @end_dt as end_dt;                 
+    select @start_ds as start_ds, @end_ds as end_ds, @start_dt as start_dt, @end_dt as end_dt;
     """)
 
     context.load()
@@ -2672,7 +2715,7 @@ def test_plan_min_intervals_adjusted_for_downstream(tmp_path: Path):
       ),
       start '2020-01-01',
       cron '@hourly'
-    );                        
+    );
 
     select @start_dt as start_dt, @end_dt as end_dt;
     """)
@@ -2681,11 +2724,11 @@ def test_plan_min_intervals_adjusted_for_downstream(tmp_path: Path):
     MODEL (
       name sqlmesh_example.two_hourly_model,
       kind INCREMENTAL_BY_TIME_RANGE (
-        time_column start_dt        
+        time_column start_dt
       ),
       start '2020-01-01',
       cron '0 */2 * * *'
-    );                        
+    );
 
     select start_dt, end_dt from sqlmesh_example.hourly_model where start_dt between @start_dt and @end_dt;
     """)
@@ -2694,11 +2737,11 @@ def test_plan_min_intervals_adjusted_for_downstream(tmp_path: Path):
     MODEL (
       name sqlmesh_example.unrelated_monthly_model,
       kind INCREMENTAL_BY_TIME_RANGE (
-        time_column start_dt        
+        time_column start_dt
       ),
       start '2020-01-01',
       cron '@monthly'
-    );                        
+    );
 
     select @start_dt as start_dt, @end_dt as end_dt;
     """)
@@ -2711,7 +2754,7 @@ def test_plan_min_intervals_adjusted_for_downstream(tmp_path: Path):
       ),
       start '2020-01-01',
       cron '@daily'
-    );                        
+    );
 
     select start_dt, end_dt from sqlmesh_example.hourly_model where start_dt between @start_dt and @end_dt;
     """)
@@ -2724,7 +2767,7 @@ def test_plan_min_intervals_adjusted_for_downstream(tmp_path: Path):
       ),
       start '2020-01-01',
       cron '@weekly'
-    );                        
+    );
 
     select start_dt, end_dt from sqlmesh_example.daily_model where start_dt between @start_dt and @end_dt;
     """)

tests/dbt/test_model.py

@@ -5,6 +5,7 @@
 from pathlib import Path
 
 from sqlglot import exp
+from sqlglot.errors import SchemaError
 from sqlmesh import Context
 from sqlmesh.core.model import TimeColumn, IncrementalByTimeRangeKind
 from sqlmesh.core.model.kind import OnDestructiveChange, OnAdditiveChange
@@ -579,3 +580,40 @@ def test_load_microbatch_with_ref_no_filter(
         context.render(microbatch_two_snapshot_fqn, start="2025-01-01", end="2025-01-10").sql()
         == 'SELECT "microbatch"."cola" AS "cola", "microbatch"."ds" AS "ds" FROM "local"."main"."microbatch" AS "microbatch"'
     )
+
+
+def test_dbt_jinja_macro_undefined_variable_error(create_empty_project):
+    project_dir, model_dir = create_empty_project()
+
+    macros_dir = project_dir / "macros"
+    macros_dir.mkdir()
+
+    # the execute guard in the macro is so that dbt won't fail on the manifest loading earlier
+    macro_file = macros_dir / "my_macro.sql"
+    macro_file.write_text("""
+{%- macro select_columns(table_name) -%}
+  {% if execute %}
+    {%- if target.name == 'production' -%}
+        {%- set columns = run_query('SELECT column_name FROM information_schema.columns WHERE table_name = \'' ~ table_name ~ '\'') -%}
+    {%- endif -%}
+    SELECT {{ columns.rows[0][0] }} FROM {{ table_name }}
+  {%- endif -%}
+{%- endmacro -%}
+""")
+
+    model_file = model_dir / "my_model.sql"
+    model_file.write_text("""
+{{ config(
+    materialized='table'
+) }}
+
+{{ select_columns('users') }}
+""")
+
+    with pytest.raises(SchemaError) as exc_info:
+        Context(paths=project_dir)
+
+    error_message = str(exc_info.value)
+    assert "Failed to update model schemas" in error_message
+    assert "Could not render or parse jinja for" in error_message
+    assert "Undefined macro/variable: 'columns' in macro: select_columns" in error_message