Repository navigation
Conversation
The `Spanned::span` implementations are plain unbounded recursion — one stack frame per AST node. Computing the span of a deeply nested AST (a deep expression tree, nested subqueries, or a long left-associative boolean chain) therefore recurses as deep as the AST and can overflow the thread stack, aborting the process. A query can parse successfully — the parser is already guarded — and then overflow later when its span is computed (e.g. during query planning by a downstream consumer). The parser already opts into stack-overflow protection behind the `recursive-protection` feature (`recursive::recursive`, backed by `stacker::maybe_grow`); the span walk was simply never given the same treatment. Annotate the recursive `span()` implementations for the container nodes (`Expr`, `SetExpr`, `Query`, `Select`, `SelectItem`, `TableFactor`, `Statement`) so at least one node on every deep-recursion cycle grows the stack on demand. No new dependency and a no-op when the feature is disabled. Adds a regression test that walks a deeply nested AST on a small thread stack: it overflows without the annotations and completes with them. Signed-off-by: fmcmac <[email protected]>
fmcmac
force-pushed
the
upstream-span-recursive-protection
branch
from
July 10, 2026 02:04
ac9702d to
b7cbd09
Compare
Contributor
|
@fmcmac could you take a look at the ci failures when you have the time? |
Resolve 7 clippy errors that fail the lint CI job under a newer toolchain: - useless_borrows_in_formatting: drop redundant `&` in write!/format! args (ast/ddl.rs, ast/query.rs, ast/mod.rs, parser/mod.rs x2) - needless_return_with_question_mark: drop redundant `?` on returned Err/parser_err! in parser/mod.rs x2 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The lib-only fixes let clippy proceed past the library to --all-targets, surfacing the same lint in examples and tests under clippy 0.1.97: - examples/cli.rs: drop redundant `&` in println!/panic! args - tests/sqlparser_postgres.rs x2, tests/sqlparser_common.rs x3: drop redundant `&str_op` in format! args Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
2 of 6 tasks
Contributor
|
Is there a real issue / usecase (like a sql query you parsed) that this is fixing? |
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
marked this pull request as draft
September 3, 2026 14:30
LucaCappelletti94
added
the
waiting on contributor
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
4 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.
What
The
Spanned::spanimplementations are plain unbounded recursion — one stack frame per AST node. Computing the span of a deeply nested AST (a deep expression tree, nested subqueries, or a long left-associative boolean chain) recurses as deep as the AST and can overflow the thread stack, aborting the process.Notably, a query can parse successfully — the parser is already guarded by the
recursive-protectionfeature — and then overflow later, when its span is computed by a downstream consumer (e.g. during query planning). We hit this in production via a downstream planner that calls.span()on user-supplied ASTs.Fix
The parser already opts into stack-overflow protection behind the
recursive-protectionfeature (recursive::recursive, backed bystacker::maybe_grow); the span walk was simply never given the same treatment. This annotates the recursivespan()implementations for the container nodes —Expr,SetExpr,Query,Select,SelectItem,TableFactor,Statement— so at least one node on every deep-recursion cycle grows the stack on demand.recursivecrate and feature already exist).recursive-protectionis disabled (same#[cfg_attr(...)]gating the parser uses).stacker::maybe_growto fire; these seven cover the deep axes (expression nesting, set-op/subquery/join nesting, statement nesting).Test
Adds a regression test that builds a deeply nested AST and walks its span on a deliberately small (512 KiB) thread stack: it overflows without the annotations and completes with them.