diff --git a/src/ol_dbt_cli/ol_dbt_cli/lib/sql_parser.py b/src/ol_dbt_cli/ol_dbt_cli/lib/sql_parser.py index c7b38fec7..0bcaebcc3 100644 --- a/src/ol_dbt_cli/ol_dbt_cli/lib/sql_parser.py +++ b/src/ol_dbt_cli/ol_dbt_cli/lib/sql_parser.py @@ -158,9 +158,27 @@ def _render_jinja(sql: str) -> tuple[str, list[str], list[str], dict[str, str], # The leading comma reconnects to the preceding CTE: these injector macros # (e.g. deduplicate_raw_table) emit leading-comma CTEs and always follow a # `source`/base CTE, which itself carries no trailing comma. + # + # The same "placeholder alone on its line, trailing comma" shape also occurs when + # a macro call is simply the *first argument* of a multi-line function call, e.g. + # ``coalesce(\n {{ macro(...) }},\n regexp_extract(...)\n)``. There the + # placeholder is not a CTE at all — it is nested inside the call's parens — and + # rewriting it as a CTE definition corrupts the argument list. sqlglot's + # error_level=IGNORE then degrades gracefully to a stray ``select 1``, so the + # model appears to parse cleanly but yields zero output columns (a false-positive + # "columns missing from SQL" error). Distinguish the two shapes by inspecting the + # nearest preceding non-whitespace character: a genuine CTE-position macro always + # immediately follows the closing ``)`` of the previous CTE's body, whereas a macro + # used as a function argument follows an opening ``(`` or a sibling-argument ``,``. + def _replace_trailing_comma_macro(m: re.Match[str]) -> str: + prefix = rendered[: m.start()].rstrip() + if prefix.endswith(")"): + return ", __jinja_cte__ as (select 1)," + return m.group(0) # function-argument position — leave the bare placeholder as-is + rendered = re.sub( r"(?m)^[^\S\r\n]*(?:__macro__|__undefined__)[^\S\r\n]*,[^\S\r\n]*$", - ", __jinja_cte__ as (select 1),", + _replace_trailing_comma_macro, rendered, ) # Block-level macro injections (macros that inject CTE SQL fragments) render as diff --git a/src/ol_dbt_cli/tests/test_sql_parser.py b/src/ol_dbt_cli/tests/test_sql_parser.py index 252325014..682b7fb48 100644 --- a/src/ol_dbt_cli/tests/test_sql_parser.py +++ b/src/ol_dbt_cli/tests/test_sql_parser.py @@ -353,6 +353,36 @@ def test_multiline_function_over_macro_keeps_alias(self) -> None: assert result.parse_error is None assert "program_readable_id" in result.output_columns + def test_macro_as_first_function_argument_not_treated_as_cte(self) -> None: + r"""A macro call as a function argument must not be treated as a CTE placeholder. + + `coalesce(\n {{ macro(...) }},\n regexp_extract(...)\n)` renders the + macro call to `__macro__,` alone on its line — the exact same shape the + trailing-comma-CTE rule (for `{{ deduplicate_raw_table(...) }},` between two + named CTEs) matches on. But here the placeholder is nested inside `coalesce(`'s + parens, not sitting between two top-level CTEs. Rewriting it as + `, __jinja_cte__ as (select 1),` corrupts the argument list; sqlglot's + error_level=IGNORE then silently degrades to a stray `select 1`, so the model + "parses" but yields zero output columns — a false-positive "columns missing + from SQL" error (tfact_chatbot_events regression). The two shapes are told + apart by the character preceding the placeholder line: a genuine CTE-position + macro always follows the closing `)` of the previous CTE, while a + function-argument macro follows the call's opening `(`. + """ + sql = ( + "with report as (select 1 as event_id, 'x' as event_value)\n" + "select\n" + " report.event_id\n" + " , coalesce(\n" + " {{ json_query_string('event_value', \"'$.thread_id'\") }},\n" + " regexp_extract(report.event_value, 'x', 1)\n" + " ) as thread_id\n" + "from report" + ) + result = parse_model_sql("my_model", sql) + assert result.parse_error is None + assert result.output_columns == {"event_id", "thread_id"} + def test_var_in_where_clause_parseable(self) -> None: """A model with '{{ var(...) }}' in WHERE must parse without error.""" sql = (