Repository navigation
Parser: fix exponential parse time on chains of (x)-> and nested POSITION calls - #18
Open
moshap-firebolt wants to merge 2 commits into
Open
moshap-firebolt wants to merge 2 commits into
moshap-firebolt wants to merge 2 commits into
Conversation
) (cherry picked from commit e790703) Conflict: this branch keeps its own `IN (` cache (a set of positions) rather than upstream's later map, so only the POSITION field is added. Dropped parse_position_in_connect_by_list_after_state_change: its SQL starts an IN list with a parenthesized subquery, which needs upstream apache#2586 and fails to parse here with or without this change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
In dialects with lambda functions, `try_parse_lambda` speculatively parses `(params) -> body` and rolls back if the body fails. The fallback parses `(x)` as a nested expression and `->` as the binary `Arrow` operator, whose right side starts at the same `(` and re-attempts the identical lambda parse, so each level of `(x)->(x)->...` costs twice the one below it. `SELECT (x)->...1 +` with 16 arrows took 98ms and doubles per arrow. Memoize the failed positions, as `parse_in` and `parse_table_factor` already do for the same shape. A cached failure yields the same fallback as re-running the parse, so behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two speculative parses that roll back and retry the same tail, so each level doubles the work:
SELECT (x)->(x)->...1 +with 24 arrows took 23.7s. A failed(x) -> bodyattempt falls back to(x)and the binary->, whose right side retries the same lambda. Failed lambda positions are now memoized, keyed by parser state like the POSITION cache. A cached failure takes the same fallback, so results are unchanged.SELECT POSITION(POSITION(...1))with 24 levels took 26s in every dialect. This is a cherry-pick of upstream Parser: fix exponential parse time on nested POSITION calls apache/datafusion-sqlparser-rs#2619. It drops one upstream test whose SQL needs Generic: Parse IN lists that start with a parenthesized subquery apache/datafusion-sqlparser-rs#2586, which this branch doesn't have.Both shapes now parse in about 2ms. New tests:
parse_lambda_arrow_chain_no_exponential_blowupandparse_lambda_vs_arrow_operator_dispatch, plus apache#2619's own tests. The full suite, fmt and clippy are clean.🤖 Generated with Claude Code
Note
Medium Risk
Touches core expression parsing dispatch; memoization is designed to preserve fallback behavior but wrong cache keys could change how ambiguous SQL is classified.
Overview
Fixes exponential parse time on two speculative-parse patterns by memoizing failed attempts at the same token index (and parser state, so
CONNECT BY/PRIORbehavior stays correct).For lambda arrow syntax in dialects that support it, chained
(x)->(x)->...with an invalid tail no longer re-tries the same speculative lambda parse on every binary-->fallback. Failed positions are recorded infailed_lambda_positionsand short-circuit to the sameOk(None)path as a fresh failure.For
POSITION(... IN ...), nestedPOSITION(POSITION(...))no longer re-parses theINform on every outer function-call fallback. Failures are cached infailed_position_syntax_positionsbefore delegating to ordinaryparse_function.Adds regression tests for time bounds on deep chains and for unchanged AST dispatch (lambda vs arrow operator,
POSITIONspecial form vs function call).Reviewed by Cursor Bugbot for commit 8ffe445. Bugbot is set up for automated code reviews on this repo. Configure here.