fix(ol_dbt_cli): bound broken-column-macro collapse to prevent cross-CTE overrun (#2442)

* fix(ol_dbt_cli): bound broken-column-macro collapse to prevent cross-CTE overrun

sql_parser.py's Jinja-macro-tail collapse used an unbounded lazy loop to find
the next `) as alias` after a placeholder. A bare (unaliased) macro call
repeated in a GROUP BY list has no real terminal, so the search ran past the
GROUP BY, the CTE's closing paren, and the outer SELECT keyword before seizing
on an unrelated downstream alias -- corrupting the parsed SQL and producing
false yaml_sql_sync/broken_ref_columns errors on
marts__combined_program_enrollment_detail (which blocked this PR's CI after
rebasing onto the new dbt PR CI gate). Bound the continuation to 6 lines, which
still covers the two existing orphaned-tail regression tests, and make the
leading ')' before 'as' optional so a plain multi-line macro+alias (no stray
tokens) resolves on the very next line instead of searching further.

* fix(ol_dbt_cli): avoid invalid self-referential CTE in regression test

The new test's CTE was named 'combined' and its own body did 'from combined',
an invalid recursive self-reference without WITH RECURSIVE. Rename the CTE's
underlying source to raw_source so the test SQL is valid while still
exercising the same placeholder-collapsing behavior.

Author: blarghmatey

src/ol_dbt_cli/ol_dbt_cli/lib/sql_parser.py

@@ -197,15 +197,29 @@ def _render_jinja(sql: str) -> tuple[str, list[str], list[str], dict[str, str],
 #   , __jinja__) as array (varchar)), ', '   <- orphaned tail (array(varchar) is a cast)
 #   ) as course_level                        <- real alias, must be last token on its line
 #
-# The regex uses a tempered greedy token `(?:(?!<next_jinja_col>)[^\n]*\n)*?` to
-# avoid consuming past the next broken-column placeholder.
+# The regex uses a tempered greedy token `(?:(?!<next_jinja_col>)[^\n]*\n){0,6}?` to
+# avoid consuming past the next broken-column placeholder. The repeat count is
+# capped (rather than unbounded `*?`) because a *bare* placeholder that never
+# gets an alias at all — e.g. the same macro call repeated, unaliased, inside a
+# GROUP BY list — has no real terminal to find. Left unbounded, the lazy search
+# happily scans past the GROUP BY, the CTE's closing paren, and even a `select`
+# keyword to seize on some unrelated downstream `) as alias`, corrupting the
+# SQL by deleting everything in between. Real broken-macro tails (see the two
+# test cases below) resolve within 1-4 lines, so a small bound safely covers
+# genuine cases while making runaway cross-CTE matches structurally impossible:
+# past the bound the regex simply fails to match and the bare placeholder is
+# left as-is (a harmless unresolved identifier) instead of eating the file.
 # Matches both ``__jinja__`` (regex path) and ``__macro__`` (Jinja2 path) placeholders.
 _BROKEN_COL_PLACEHOLDER = r"(?:__jinja__|__macro__|__undefined__)"
 _BROKEN_COL_RE = re.compile(
     rf"([ \t]*,[ \t]*){_BROKEN_COL_PLACEHOLDER}(?![ \t]+as\b)"  # column-sep + placeholder, no direct alias
     r"[^\n]*\n"  # rest of the first (broken) line
-    rf"(?:(?![ \t]*,[ \t]*{_BROKEN_COL_PLACEHOLDER})[^\n]*\n)*?"  # optional continuation lines
-    r"[ \t]*\)[ \t]+as[ \t]+(\w+)[ \t]*(?:--[^\n]*)?(?:\n|$)",  # ) as alias — must be last token on line
+    rf"(?:(?![ \t]*,[ \t]*{_BROKEN_COL_PLACEHOLDER})[^\n]*\n){{0,6}}?"  # bounded continuation lines
+    r"[ \t]*\)?[ \t]*as[ \t]+(\w+)[ \t]*(?:--[^\n]*)?(?:\n|$)",  # [)] as alias — must be last token on line.
+    # The ')' is optional: a plain macro call (e.g. a UDF-style {{ macro(...) }})
+    # followed by `as alias` alone on the next line has no stray tokens to
+    # collapse, so the lazy loop must terminate there instead of overrunning
+    # into an unrelated later `) as ...` several columns/CTEs downstream.
 )
 
 

src/ol_dbt_cli/tests/test_sql_parser.py

@@ -270,6 +270,40 @@ def test_broken_column_macro_multiline_case(self) -> None:
         assert "item_id" in result.output_columns
         assert "item_types" in result.output_columns
 
+    def test_broken_column_regex_stops_at_plain_multiline_alias(self) -> None:
+        """A plain macro call aliased on the next line must not swallow later columns.
+
+        Unlike the orphaned-tail cases above, ``{{ macro(...) }}`` here has no stray
+        tokens trailing it — the very next line is just ``as alias``. The collapse
+        regex must terminate there instead of lazily overrunning into unrelated
+        later columns (and even later CTEs) looking for the next ``) as ...``.
+        """
+        sql = (
+            "with combined as (\n"
+            "    select\n"
+            "        raw_source.platform_name\n"
+            "        , {{ generate_hash_id('cast(raw_source.user_id as varchar)') }}\n"
+            "            as user_hashed_id\n"
+            "        , raw_source.user_id\n"
+            "        , count(distinct raw_source.course_id) as unique_courses\n"
+            "    from raw_source\n"
+            "    group by\n"
+            "        raw_source.platform_name\n"
+            "        , {{ generate_hash_id('cast(raw_source.user_id as varchar)') }}\n"
+            "        , raw_source.user_id\n"
+            ")\n"
+            "select\n"
+            "    combined.platform_name\n"
+            "    , combined.user_hashed_id\n"
+            "    , combined.unique_courses\n"
+            "from combined"
+        )
+        result = parse_model_sql("my_model", sql)
+        assert result.parse_error is None
+        assert "platform_name" in result.output_columns
+        assert "user_hashed_id" in result.output_columns
+        assert "unique_courses" in result.output_columns
+
     def test_var_in_where_clause_parseable(self) -> None:
         """A model with '{{ var(...) }}' in WHERE must parse without error."""
         sql = (