Skip to content

IN LIST: centralize primitive filters and optimize Decimal128 - #24283

Draft
geoffreyclaude wants to merge 3 commits into
apache:mainfrom
geoffreyclaude:codex/in-list-primitive-selector-decimal
Draft

IN LIST: centralize primitive filters and optimize Decimal128#24283
geoffreyclaude wants to merge 3 commits into
apache:mainfrom
geoffreyclaude:codex/in-list-primitive-selector-decimal

Conversation

@geoffreyclaude

@geoffreyclaude geoffreyclaude commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

#23014 adds fast direct comparisons for short primitive IN lists. For longer lists, DataFusion uses a bitmap, a hash set, or another specialized filter when one exists, and otherwise uses the general filter.

#24102 needs to make that same choice after reading FixedSizeBinary values as same-width primitive keys. Today, the choice is part of the native primitive path, so #24102 would have to repeat its limits and fallback rules.

This PR moves that choice beside the primitive filters and makes it reusable. The limits count only non-null list values. It also combines the separate Int32, Int64, UInt32, and UInt64 hash-set filters into one implementation. Other existing primitive types keep the same behavior.

The only new lookup path is for Decimal128. Lists with at most four non-null values still use direct comparisons. Larger lists now use a hash set over the stored i128 values instead of the general Arrow filter. Values are not rescaled, and the existing precision, scale, and null rules are unchanged. #24102 also uses this path for 16-byte FixedSizeBinary values.

What changes are included in this PR?

  • Makes primitive filter selection reusable by IN LIST: reuse primitive filters for FixedSizeBinary #24102.
  • Replaces four integer-specific hash-set filters with PrimitiveHashSetFilter<T> and reuses the shared SQL result builder.
  • Uses that hash set for Decimal128 lists with more than four non-null values.
  • Adds four Decimal128 benchmark cases at list sizes 5 and 64.

Are these changes tested?

Tests cover the direct-comparison boundary, an all-null list, and the larger Decimal128 path with hits, misses, a null in the list, IN, NOT IN, dictionary inputs, and compatible precision/scale metadata.

Are there any user-facing changes?

No. SQL results and public APIs are unchanged.

Benchmark snapshot

The benchmark uses list size 5, the first size above the direct-comparison limit, and a larger size of 64. Each size is measured with 0% and 50% matches. Filter construction is outside the timed loop.

A previous automated run measured these code states before this work was split from #24102. The split did not change the Decimal128 implementation on either side.

Compared baselines: #23014 -> this PR

Benchmark Before After Change
primitive/decimal128/large_list/list=5/match=0% 18.1 us 15.2 us -16.0% (1.19x faster)
primitive/decimal128/large_list/list=5/match=50% 38.6 us 19.2 us -50.3% (2.01x faster)
primitive/decimal128/large_list/list=64/match=0% 17.6 us 15.2 us -13.6% (1.16x faster)
primitive/decimal128/large_list/list=64/match=50% 37.1 us 23.5 us -36.7% (1.58x faster)

geoffreyclaude and others added 3 commits August 12, 2026 12:02
Move primitive strategy selection beside the filter implementations. Primitive arrays and representation adapters share the same branchless cutoffs and bitmap/hash-set fallbacks. Reuse PrimitiveHashSetFilter and SQL result construction across native integer filters.
Route Decimal128 lists above the branchless cutoff through PrimitiveHashSetFilter, replacing the ArrayStaticFilter fallback. Add tests for SQL null propagation, NOT IN, dictionary needles, and compatible decimal metadata.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.38710% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.28%. Comparing base (426b351) to head (a1a22b1).
⚠️ Report is 95 commits behind head on main.

Files with missing lines Patch % Lines
...l-expr/src/expressions/in_list/primitive_filter.rs 68.66% 29 Missing and 18 partials ⚠️
.../physical-expr/src/expressions/in_list/strategy.rs 0.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24283      +/-   ##
==========================================
+ Coverage   80.91%   81.28%   +0.36%     
==========================================
  Files        1102     1110       +8     
  Lines      377102   385235    +8133     
  Branches   377102   385235    +8133     
==========================================
+ Hits       305143   313143    +8000     
+ Misses      53769    53596     -173     
- Partials    18190    18496     +306     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants