Skip to content

Apply recursive-protection to the Spanned::span walk#2400

Open
fmcmac wants to merge 1 commit into
apache:mainfrom
fmcmac:upstream-span-recursive-protection
Open

Apply recursive-protection to the Spanned::span walk#2400
fmcmac wants to merge 1 commit into
apache:mainfrom
fmcmac:upstream-span-recursive-protection

Conversation

@fmcmac

@fmcmac fmcmac commented Jul 10, 2026

Copy link
Copy Markdown

What

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) 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-protection feature — 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-protection feature (recursive::recursive, backed by stacker::maybe_grow); the span walk was simply never given the same treatment. This annotates 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 (the recursive crate and feature already exist).
  • A no-op when recursive-protection is disabled (same #[cfg_attr(...)] gating the parser uses).
  • Annotating one node per cycle is sufficient for stacker::maybe_grow to 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.

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 <francis@zuru.com>
@fmcmac fmcmac force-pushed the upstream-span-recursive-protection branch from ac9702d to b7cbd09 Compare July 10, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant