Skip to content

Parser: align additional recursive paths in the recursion limit - #2440

Open
fmcmac wants to merge 1 commit into
apache:mainfrom
fmcmac:recursion-counter-gaps
Open

fmcmac wants to merge 1 commit into
apache:mainfrom
fmcmac:recursion-counter-gaps

Conversation

@fmcmac

@fmcmac fmcmac commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Dialects: Snowflake, MySQL, Generic

Nested shapes avoid the recursion limit, overflowing the stack instead of returning RecursionLimitExceeded:

  • MATCH_RECOGNIZE(PATTERN ((((a)))) ...)
  • JSON_TABLE(... COLUMNS(NESTED PATH '$' COLUMNS(...)))
  • Snowflake COPY_OPTIONS=(a=(a=(...)))
  • Snowflake SELECT * FROM t JOIN t JOIN t ..., without parentheses

Added recursion guard for parse_pattern, parse_json_table_column_def, parse_key_value_options and the recursive call in parse_joins.

The depth tests overflow on main. Additional tests check that a flat 100-way PATTERN alternation and nested parenthesised joins still parse.

This PR was written with Claude Code. I have read and reviewed every line.

@LucaCappelletti94 LucaCappelletti94 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the changes are AI-slopped documentation. Please only include actually necessary documentation, read it, and not just greenlight AI generated documentation.

The code itself looks okay.

@alamb

alamb commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Marking as draft as I think this PR is no longer waiting on feedback and I am trying to make it easier to find PRs in need of review. Please mark it as ready for review when it is ready for another look

@alamb
alamb marked this pull request as draft September 3, 2026 14:29
@alamb

alamb commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for the contribution @fmcmac and the review @LucaCappelletti94

parse_pattern, parse_json_table_column_def and parse_key_value_options
recurse on nested input without a depth guard. So does parse_joins, on
Snowflake join chains without parentheses. Deep enough input overflows
the stack. The first 3 now take a guard on entry. parse_joins takes one
on its recursive call.

parse_pattern now reads `a | b | c` in a loop rather than recursing once
per alternative, so the guard counts nesting, not alternatives.

Generated-by: Claude Code
@fmcmac
fmcmac force-pushed the recursion-counter-gaps branch from 56d8335 to 7dcc189 Compare October 30, 2026 02:20
@fmcmac fmcmac changed the title Close the remaining gaps in the parser recursion counter Parser: align additional recursive paths in the recursion limit Oct 30, 2026
@fmcmac
fmcmac marked this pull request as ready for review October 30, 2026 02:20
@fmcmac

fmcmac commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Mea culpa @LucaCappelletti94, I have removed the comments.

Rebased onto main. #2411 and #2422 cover the data type and interval paths, so I dropped those parts. I removed the parse_prefix change, because prefix caching covers the slowdown and the change rejected valid SQL. Tests now align with the existing style.

@github-actions github-actions Bot added the MySQL label Oct 30, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.55%. Comparing base (50813ee) to head (7dcc189).

Files with missing lines Patch % Lines
src/parser/mod.rs 80.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2440      +/-   ##
==========================================
+ Coverage   82.54%   82.55%   +0.01%     
==========================================
  Files          42       42              
  Lines       35553    35555       +2     
  Branches    35553    35555       +2     
==========================================
+ Hits        29346    29354       +8     
  Misses       2672     2672              
+ Partials     3535     3529       -6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MySQL Snowflake waiting on contributor The review needs further refinements by its author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants