Skip to content

Prune large IN sets against each zone's value range - #107

Merged
ChristopherWirt merged 2 commits into
developfrom
cwirt/fix-issue-101
Nov 10, 2026
Merged

ChristopherWirt merged 2 commits into
developfrom
cwirt/fix-issue-101

Conversation

@ChristopherWirt

@ChristopherWirt ChristopherWirt commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Zone pruning of col IN (...) stops working once the list has more than 16 values:

  1. Up to 16 values, each value gets its own max < v OR min > v test, so zones that fall between values are pruned.
  2. Beyond 16, the falsifier only checks the list's overall [min, max]. A zone in a gap between two keys can hold no member of the set, yet it is still scanned.
  3. The falsifier also only recognised a literal list. After optimisation the list becomes a PreparedSet, so an optimised IN produced no falsifier. Spark worked around this by spelling out explicit min <= col <= max bounds.

Change

  • Add SetDisjoint, an internal stats expression. It holds the set's sorted, de-duplicated non-null members and binary-searches each zone's [min, max] for a member: O(log n) per zone, and the expression stays compact.
  • list_contains_falsify now reads the list from either a Literal or a PreparedSet. For more than 16 values it emits SetDisjoint for bool, primitive, decimal, utf8 and binary lists. Other dtypes keep the previous overall-bounds fallback.
  • The existing non-NaN guards are kept, and missing or null bounds never prune a zone.
  • The Spark explicit IN bounds (inBounds/orderKey) are removed. The Rust rewrite now produces the same bounds or tighter ones.

Tests

New zone_map tests cover:

  • the 16/17-value boundary
  • 80 integer keys and 80 string keys
  • duplicates and nulls in the list
  • missing stats
  • NaN and signed zero, with and without NaN-count stats
  • literal and optimised (PreparedSet) expressions

A stress test with 16,384 members over 8,192 zones checks that pruning matches actual row membership.

Validation

  • cargo nextest run -p vortex-expr -p vortex-layout -E 'test(stats::rewrite) | test(layouts::zoned::zone_map)': 61 passed
  • cargo clippy -p vortex-expr -p vortex-layout --all-targets --all-features -- -D warnings: clean
  • ./gradlew :vortex-spark-3.5_2.12:test --tests SparkFilterToVortexExpressionTest: passed

Follow-ups (not in this PR)

  • Date/Timestamp (extension) lists still use overall bounds only.
  • SetDisjoint compares Scalars per zone. A typed fast path, and sharing the sorted set with PreparedSet's haystack, could be added if benchmarks warrant it.

Fixes #101

🤖 Generated with Claude Code

ChristopherWirt added a commit that referenced this pull request Nov 10, 2026
## Summary

Fix the shared Linux musl builder failure on `develop` and PRs #103,
#105, #106, #107, and #108 after the upstream lazy-constructor update.

## Changes

Normalize lists rebuilt from list views through the same `reset_offsets`
path as ordinary lists before forwarding their children to layout
writers. Rebuilding a sliced list view can now leave a lazy elements
slice; a flat child writer cannot serialize that expression. This
preserves the existing referenced-element bounds and resolves the slice
without forcing unrelated encodings into canonical arrays.

Keep the existing nullable sliced-list-view regression and add the
non-nullable case.

## CI Failure Analysis

### Status

Pre-existing base-branch regression, independent of the individual PR
changes. The same failure appears on [develop run
42649358621](https://github.com/quantcast/vortex/actions/runs/42649358621)
and every completed failing PR builder inspected (#105–#108).

### Failed Jobs

- `Rust tests (linux-musl)`: test failure in
`layouts::list::reader::tests::sliced_list_chunks_round_trip::case_4_list_view_chunks`.

### Relevant Log Output

```text
Error: Array vortex.slice is not registered for serialization
```

### Correlation With PR Changes

The upstream merge makes array slicing lazy. The list-view branch of
`canonicalize_to_list_parts` forwarded a rebuilt list's lazy elements
directly, while the ordinary-list branch already normalized them. The
Spark PRs do not change this writer; #107 changes separate zone-pruning
code.

### Recommended Next Step

Merge this shared fix into `develop`. The same patch is being applied to
the affected open PR branches so their new builders include the fix
immediately.

## Validation

- Reproduced the exact failing test on unmodified current `develop`
(exit 100, same serialization error).
- `cargo nextest run -p vortex-layout -E 'test(layouts::list::)'`: all
161 tests passed after the fix, including the original failing case and
new non-nullable case.
- `cargo clippy -p vortex-layout --all-targets --all-features -- -D
warnings`: passed.
- Pinned nightly Rust formatting and `git diff --check`: passed.

Local validation ran on macOS; the fresh Linux musl builders will verify
the runner-specific job.

Signed-off-by: Christopher Wirt <cwirt@quantcast.com>
ChristopherWirt and others added 2 commits November 10, 2026 10:22
Signed-off-by: Christopher Wirt <cwirt@quantcast.com>
The Spark IN bounds only existed because the stats rewrite could not see
a prepared set; it now can, so they duplicated the Rust pruning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Christopher Wirt <cwirt@quantcast.com>
@ChristopherWirt
ChristopherWirt merged commit b2c429a into develop Nov 10, 2026
1 check passed
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.

IN filters of more than 16 values prune zones only by the set's overall min and max

1 participant