Skip to content

fix(jinja): route recursive macro calls through the registry wrapper - #6139

Open
raams95 wants to merge 1 commit into
SQLMesh:mainfrom
raams95:fix/jinja-recursive-macro-return
Open

raams95 wants to merge 1 commit into
SQLMesh:mainfrom
raams95:fix/jinja-recursive-macro-return

Conversation

@raams95

@raams95 raams95 commented Oct 13, 2026

Copy link
Copy Markdown

Description

Fixes #5341. The macro in that issue calls itself to walk nested struct fields, then returns the collected list with return(). Under SQLMesh it only ever got the fields from the first recursive call. As the issue thread suggests, this isn't specific to BigQuery or to get_columns_in_relation. Any recursive macro that uses return() hits it. Here's a minimal repro with no adapter involved:

{% macro flatten(items) %}
  {% set result = [] %}
  {% for item in items %}
    {% if item is iterable and item is not string %}
      {% do result.extend(flatten(item)) %}
    {% else %}
      {% do result.append(item) %}
    {% endif %}
  {% endfor %}
  {{ return(result) }}
{% endmacro %}

{{ flatten([1, [2, 3], 4, [5, [6]]]) }}

dbt renders [1, 2, 3, 4, 5, 6]. SQLMesh renders [2, 3].

Root cause. JinjaMacroRegistry parses each macro under its own name. _MacroWrapper calls the macro and catches MacroReturnVal, which is how return() works. Inside the macro's body, though, Jinja resolves the macro's own name to the raw template macro, not to the wrapper in the context. So the inner recursive call's return() raises MacroReturnVal straight through the outer call. The outer macro stops at its first recursive call and returns the inner call's value.

Fix. Each macro is now defined under an internal name (sqlmesh_macro__<name>), and the wrapper looks it up by that name. When the body refers to the macro's own name, Jinja resolves it from the context like any other macro reference, so recursive calls go through _MacroWrapper and each call handles its own return(). dbt does the same thing: it renames macros to dbt_macro__<name>. Undefined variable errors still show the macro's own name (extract_error_details strips the prefix).

This also replaces the private macro workaround (_to_non_private_macro_def). That workaround renamed every Name call in a private macro's body to the macro itself, not just recursive calls. So a private macro that called anything else by name broke. For example, {% macro _helper() %}{{ other_macro() }}{% endmacro %} hit RecursionError, and {{ range(3) }} inside a private macro raised TypeError: macro '...' takes not more than 0 argument(s). With the internal name, private macros are exported without rewriting their bodies.

Rendered macro definitions, data_hash_values and serialization don't change. Only the template that gets compiled at render time does, so model fingerprints are unaffected.

Test Plan

  • Added test_macro_return_recursive (the repro above) and test_macro_registry_render_private_macro_calling_other_functions to tests/utils/test_jinja.py. On main, the first renders [2, 3] and the second raises TypeError. Both pass with this change. The existing private macro recursion test (test_macro_registry_render_private_macros) still passes.
  • pytest tests/utils tests/dbt tests/core/test_model.py tests/core/test_context.py tests/core/test_audit.py tests/core/test_test.py (slow tests included) on Windows. Everything passes except 8 tests in test_context.py/test_test.py, and those fail the same way on main on my machine (Windows path/env issues, unrelated to Jinja). That run includes test_dbt_jinja_macro_undefined_variable_error and test_jinja_macro_undefined_variable_error, which check the macro name in error messages. For the dbt tests I replaced the sushi_test/dbt_packages symlinks with directory junctions locally, since git checks them out as plain files on Windows.
  • ruff check and ruff format --check are clean on the changed files. mypy sqlmesh/utils/jinja.py only reports the errors my local environment also gives on main (missing sqlmesh._version, signal.SIGUSR1).

Checklist

  • I have run make style and fixed any issues (ran ruff + mypy on the changed files only; on Windows, mypy only shows the same environment errors as on main)
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test) (ran the related test modules listed above on Windows and compared the failures against main; I didn't run the full suite)
  • My commits are signed off (git commit -s) per the DCO

Macros were parsed under their own name, so a call to a macro from its
own body resolved to the raw Jinja macro instead of the registry's
_MacroWrapper. A return() inside the inner call therefore raised
MacroReturnVal past the outer macro, which ended the outer call early
and returned the inner call's value. Recursive dbt macros that collect
results with return() (e.g. walking nested struct fields) only produced
the result of the first recursive call.

Define each macro under an internal name and call it by that name, so
that references to the macro from within its body go through the
wrapper, like calls from any other macro. The internal name is mapped
back to the macro's own name in undefined variable error messages.

This also replaces the private macro workaround, which renamed every
call in a private macro's body to the macro itself and broke private
macros that call other macros or functions.

Fixes SQLMesh#5341

Signed-off-by: JanakiRamChittineni <341781521+raams95@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recursive Function Issue in dbt Jinja

1 participant