[WIP] feat(array): sorted-merge fast path for list_contains IN-lists - #9778
Draft
NAVEENKUMARKR777 wants to merge 2 commits into
Draft
[WIP] feat(array): sorted-merge fast path for list_contains IN-lists#9778NAVEENKUMARKR777 wants to merge 2 commits into
NAVEENKUMARKR777 wants to merge 2 commits into
Conversation
Signed-off-by: bRong Njam <longran1989@gmail.com>
## Rationale for this change `col IN (<literal list>)` is evaluated in `ListContains::execute` via `constant_list_scalar_contains`, which runs one full-column `Eq` comparison per literal element and `Or`-reduces them together -- `O(list_len * column_len)`. Issue vortex-data#9551 proposes `sorted_membership_mask`, a two-binary-search-plus-merge membership check for sorted arrays, and lists "sorted IN / NOT IN evaluation" as its first motivating use case, but nothing in the tree wires it into IN-list evaluation. When the probed column already has `Stat::IsSorted`/`Stat::IsStrictSorted` known `true` (persisted in file footers, so usually free on decoded chunks), this is a real, measured win. ## What changes are included in this PR? `try_sorted_membership_contains` in `list_contains/mod.rs`: sorts and dedups the literal list (dropping nulls, since they never contribute a match under `NullEquality::Unequal`), wraps it in a `SortedArray`, and calls `sorted_membership_mask` against the probed column. Falls back to the existing fan-out unchanged when the column isn't known sorted, the element dtype is float (NaN ordering mismatch between `Scalar::partial_cmp` and `SortedArray`'s validation), or the list has fewer than 12 elements (measured crossover -- the fan-out is faster below that). Re-sorts the literal list fresh per chunk rather than caching across chunks, since `ScalarFnVTable::execute` has no cross-chunk cache today and IN-lists are normally small enough that this is negligible next to the fan-out it replaces. Local (non-CodSpeed) numbers on an 8,192-row sorted `i64` column: | list size | fan-out | sorted-merge | |---|---|---| | 16 | 68 us | 42 us | | 64 | 298 us | 51 us | | 256 | 1.22 ms | 75 us | New `vortex-array/benches/list_contains.rs` benchmark, and 15 new tests in `list_contains/mod.rs` covering the fast path directly, dtype/length threshold fallback, and a differential suite asserting the fast path and fan-out agree on the same inputs (including nulls). This depends on vortex-data#9552 (not yet merged): the base commit adding `search_sorted::membership` is cherry-picked onto this branch so the integration can be reviewed and benchmarked now. See the discussion on vortex-data#9551 before merging. Checks run: `cargo nextest run -p vortex-array` (3564 passed), `cargo test --doc -p vortex-array`, `cargo +nightly fmt --all`, `cargo clippy -p vortex-array --all-targets --all-features` (clean). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Naveenkumar <naveenkumarkr555@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
NAVEENKUMARKR777
marked this pull request as draft
September 5, 2026 00:16
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.
Summary
col IN (<literal list>)is evaluated inListContains::executeviaconstant_list_scalar_contains, which runs one full-columnEqcomparison per literal element andOr-reduces them together —O(list_len * column_len). #9551 proposessorted_membership_mask(a two-binary-search-plus-merge membership check for sorted arrays) and lists "sortedIN/NOT INevaluation" as its first motivating use case, but nothing in the tree wires it into IN-list evaluation yet.This PR adds that integration: when the probed column already has
Stat::IsSorted/Stat::IsStrictSortedknowntrue(persisted in file footers, so usually free on decoded chunks), the literal list is sorted/deduped once and matched viasorted_membership_maskinstead of the fan-out.This depends on #9552, which isn't merged yet — I cherry-picked its commit (
1868a4b, addingsearch_sorted::membership) onto this branch so the integration itself can be reviewed and benchmarked now. Opening as[WIP]for that reason; see the discussion on #9551 (cc @joseph-isaacs) before this is mergeable on its own.What changes are included in this PR?
try_sorted_membership_containsinvortex-array/src/scalar_fn/fns/list_contains/mod.rs, called fromconstant_list_scalar_containsbefore the existing fan-out. Falls back to the fan-out unchanged when:Stat::IsSorted/IsStrictSorted, never forced — no extraO(n)computation)Scalar::partial_cmpcan't order NaN the waySortedArray's validation,NativePType::total_compare, does)ScalarFnVTable::executehas no cross-chunk cache today, and IN-lists are normally small enough thatO(list_len log list_len)per chunk is negligible next to theO(list_len * column_len)fan-out it replaces.vortex-array/benches/list_contains.rs.list_contains/mod.rs(previously zero null coverage for this code path): the fast path directly, dtype/length threshold fallback, and a differential suite asserting the fast path and fan-out agree on identical inputs, including nulls.Benchmarks
Local (non-CodSpeed, so treat as directional) numbers on an 8,192-row sorted
i64column,cargo bench -p vortex-array --bench list_contains:Below ~8–12 elements the fan-out wins outright (fixed sort overhead dominates), which is where the 12-element threshold came from.
Test plan
cargo nextest run -p vortex-array— 3564 passedcargo test --doc -p vortex-arraycargo +nightly fmt --allcargo clippy -p vortex-array --all-targets --all-features— cleanNotes for reviewers
list_contains(lit(list), column)expression tree disagrees with its own canonicalized result for a null needle row. Reproducible on the untouched fan-out too. Happy to file a separate issue if useful.🤖 Generated with Claude Code