Skip to content

fix: avoid stack overflow on deep logical filters#7510

Open
Xuanwo wants to merge 1 commit into
mainfrom
xuanwo/oss-1232-segfault-on-recursive-andor-query-processing
Open

fix: avoid stack overflow on deep logical filters#7510
Xuanwo wants to merge 1 commit into
mainfrom
xuanwo/oss-1232-segfault-on-recursive-andor-query-processing

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

This fixes OSS-1232 by preventing deeply chained AND / OR SQL filters from overflowing the Rust stack during Lance's SQL filter planning.

The immediate root cause is in Lance's side of the SQL planning path. We first cloned deeply nested sqlparser AST expressions, and then recursively converted left-deep AND / OR trees into DataFusion expressions. A user-supplied flat logical filter could therefore abort the process before Lance returned an error.

This PR is intentionally a Lance-side workaround: it moves parsed SQL expressions out of the AST instead of recursively cloning them, then flattens same-operator logical chains and rebuilds a balanced DataFusion expression tree. That protects the known vulnerable path without waiting for upstream DataFusion/sqlparser behavior to change.

A more systematic follow-up should add shared expression complexity/depth limits across SQL, Substrait, DataFusion-expression inputs, merge-insert predicates, and any later optimization / planning passes, so Lance rejects pathological expressions consistently instead of fixing individual recursive call sites one by one.

@github-actions github-actions Bot added the bug Something isn't working label Jun 29, 2026
@Xuanwo Xuanwo marked this pull request as ready for review June 29, 2026 08:39
@Xuanwo Xuanwo requested a review from wjones127 June 29, 2026 08:39
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-datafusion/src/sql.rs 57.14% 6 Missing ⚠️
rust/lance-datafusion/src/planner.rs 93.93% 1 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant