Skip to content

[WIP] feat(array): sorted-merge fast path for list_contains IN-lists - #9778

Draft
NAVEENKUMARKR777 wants to merge 2 commits into
vortex-data:developfrom
NAVEENKUMARKR777:naveenkumar/sorted-in-list-merge
Draft

[WIP] feat(array): sorted-merge fast path for list_contains IN-lists#9778
NAVEENKUMARKR777 wants to merge 2 commits into
vortex-data:developfrom
NAVEENKUMARKR777:naveenkumar/sorted-in-list-merge

Conversation

@NAVEENKUMARKR777

Copy link
Copy Markdown

Summary

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). #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 yet.

This PR adds that integration: when the probed column already has Stat::IsSorted/Stat::IsStrictSorted known true (persisted in file footers, so usually free on decoded chunks), the literal list is sorted/deduped once and matched via sorted_membership_mask instead of the fan-out.

This depends on #9552, which isn't merged yet — I cherry-picked its commit (1868a4b, adding search_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_contains in vortex-array/src/scalar_fn/fns/list_contains/mod.rs, called from constant_list_scalar_contains before the existing fan-out. Falls back to the fan-out unchanged when:
    • the column isn't known sorted (peeked via Stat::IsSorted/IsStrictSorted, never forced — no extra O(n) computation)
    • the element dtype is float (Scalar::partial_cmp can't order NaN the way SortedArray's validation, NativePType::total_compare, does)
    • the literal list has fewer than 12 elements — a measured crossover, not a guess (see benchmarks below)
  • Re-sorts the literal list fresh on every chunk rather than caching across chunks: ScalarFnVTable::execute has no cross-chunk cache today, and IN-lists are normally small enough that O(list_len log list_len) per chunk is negligible next to the O(list_len * column_len) fan-out it replaces.
  • New benchmark vortex-array/benches/list_contains.rs.
  • 15 new tests in 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 i64 column, cargo bench -p vortex-array --bench list_contains:

list size fan-out sorted-merge
16 68 µs 42 µs
64 298 µs 51 µs
256 1.22 ms 75 µs

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 passed
  • cargo test --doc -p vortex-array
  • cargo +nightly fmt --all
  • cargo clippy -p vortex-array --all-targets --all-features — clean
  • CodSpeed numbers once CI runs, to replace the local ones above

Notes for reviewers

  • AI assistance disclosure: prototyped with Claude Code, per this repo's AI assistance policy.
  • Separately (not fixed in this PR, out of scope): writing null coverage for this code path surfaced a pre-existing, unrelated framework quirk where per-row scalar access on the lazy 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

aunjgr and others added 2 commits September 4, 2026 22:48
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
NAVEENKUMARKR777 marked this pull request as draft September 5, 2026 00:16
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.

2 participants