Repository navigation
Remove the 500-token cap from the nested set operation lookahead - #2780
Merged
manticore-projects merged 1 commit intoOct 13, 2026
Conversation
isNestedSetOperationAhead() gave up after 500 tokens, so a parenthesised set operation whose first branch is longer than that was parsed as a parenthesised FROM item and failed on the set operator. Walk the token chain once instead of calling getToken(i) per position, which removes the need for a cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
edeadington
marked this pull request as ready for review
October 12, 2026 21:59
Contributor
|
Thank you for this correction, much appreciated! |
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
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.
isNestedSetOperationAhead()decides whether((SELECT ...) UNION ...)is a parenthesised set operation or a parenthesised FROM item. It scans for the)that closes the first branch but gives up after 500 tokens, so when that branch is longer the parser takes the FROM item path and fails on the set operator:The same applies to
x IN ((SELECT ...) UNION (SELECT ...)). 500 tokens is only a few lines of generated SQL.The cap was most likely added to bound performance: the scan called
getToken(i)for every position, andgetToken(i)walksitokens from the current position on each call, so an uncapped scan is quadratic.This PR walks the token chain once using the existing
skipBalancedBracketGroup()helper, so the scan is linear and needs no cap. It's also faster than the capped scan for short branches.Median parse time for the query above, same build with only the lookahead swapped:
getToken(i)scan without capOn timeouts: with the quadratic scan, removing the cap would let one lookahead blow through the default 8 s
Feature.timeOuton large inputs (16 s at 80k tokens). The linear scan stays well inside it.skipBalancedBracketGroup()also checksinterrupted, so a parse that does time out stops mid-scan, where the old loop ran on to 500 regardless.Two regression tests cover the FROM and IN cases.