Skip to content

fix: support boolean predicates in scalar segment scans - #93

Merged
yanghua merged 3 commits into
lance-format:mainfrom
Gabriel39:fix/scalar-segment-expressions
Sep 30, 2026
Merged

yanghua merged 3 commits into
lance-format:mainfrom
Gabriel39:fix/scalar-segment-expressions

Conversation

@Gabriel39

@Gabriel39 Gabriel39 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

A scoped scalar scan currently selects only a single index leaf beneath AND. A short IN list rewritten to OR therefore falls back to scanning the entire requested fragment domain, even when every branch is supported by the selected segment.

Evaluate a scoped ScalarIndexExpr using Lance's boolean evaluator and a loader pinned to the selected physical segment. This supports AND, OR, IN, and NULL-aware NOT through the existing C/C++ API, with no ABI or dependency changes.

  • Retain necessary AND conditions, require candidates for both OR branches, and never negate a partially retained subtree.
  • Preserve explicit fragment boundaries, complete predicate rechecks, deletion handling, and filtering before LIMIT/OFFSET. Unsupported or inexact candidates retain the scoped scan fallback.
  • Filters containing IS [NOT] TRUE/FALSE use the scoped fallback before planning: the pinned planner loses their NULL semantics under negation. This guard can be removed after adopting fix(index): keep negated scalar index filters correct on NULL rows lance#9568. Ordinary equality negation remains indexed.
  • Bound expression selection to 128 nodes and depth 32 before evaluation. Reuse the opened segment for every leaf instead of loading a global logical index.
  • Report unknown candidate cardinality for complement masks instead of reporting zero candidates.

Validation: the new IN regression was observed failing on the original implementation with scalar_segment_fallback_no_driver=1. After the fix, the string IN regression reads four candidate rows instead of all eight rows in its fragment domain. Tests cover BTree/Bitmap, integer/string keys, NULL and empty results, stable row IDs, deleted rows, subset/partial fragment coverage, residual pagination, other indexed columns, unsafe negation, and expression budgets.

  • cargo fmt --check
  • cargo check --locked --all-targets
  • cargo clippy --locked --all-targets -- -D warnings
  • cargo test --locked: 456 tests passed
  • cargo test --locked --test compile_and_run_test -- --ignored: 3 tests passed on implementation commit 78626b1 (before the test/documentation follow-up)

Nullable-boolean regressions cover both index types and stable-row-ID modes, positive and negated truth tests, nested AND/OR, fragment scope, pagination, and continued acceleration of NOT (key = true). The review reproducer failed with missing NULL rows before the guard was added.

Array-label integration coverage:

  • Document the existing SQL/Substrait LabelList path, required list-element schema, and canonical Lance function names. No new ABI or production execution changes are needed.
  • Add a C API Substrait regression for List membership, AND/OR, all/any labels, exact candidate counts, NULL/empty/duplicate labels, residual filtering, pagination, fragment scope, partial coverage including unindexed rows, stable row IDs, and deletion handling.
  • Explain cross-engine boundaries: NULL membership and ordered-subsequence functions must not be mapped by name alone; the pinned Lance version also treats an empty all-label query as true on NULL lists.
  • The array-label follow-up passes all 456 Rust tests, cargo check, Clippy, formatting, and diff checks. Native consumer tests passed on the preceding implementation commit; this follow-up changes only tests and documentation.

lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Sep 30, 2026
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Sep 30, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Gate recommendation: approve.

Segment-scoped AND/OR/IN and ordinary NOT preserve the explicit fragment domain and recheck the complete predicate before LIMIT/OFFSET. Boolean truth tests safely retain the scoped fallback for the pinned planner, preserving matching NULL rows.

The new Substrait LabelList regressions confirm label intersections and unions, NULL/empty semantics, partial coverage, pagination, and deletion handling with both row-ID modes. The documented cross-engine boundaries match the pinned Lance semantics. This follow-up adds tests and documentation without changing production execution or the ABI.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 30, 2026

@yanghua yanghua left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Labels

K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants