Skip to content

arrow-select: fuse inline BinaryView filter coalescing#9755

Open
ClSlaid wants to merge 2 commits intoapache:mainfrom
ClSlaid:fix-9143-binaryview-filter-coalescer
Open

arrow-select: fuse inline BinaryView filter coalescing#9755
ClSlaid wants to merge 2 commits intoapache:mainfrom
ClSlaid:fix-9143-binaryview-filter-coalescer

Conversation

@ClSlaid
Copy link
Copy Markdown
Contributor

@ClSlaid ClSlaid commented Apr 17, 2026

Summary

  • fuse the sparse inline BinaryView filter and coalescing paths so primitive columns and inline views can be appended directly without materialising an intermediate filtered RecordBatch
  • reuse optimised filter indices and null-mask handling for coalescing, while preserving the existing fallback paths for dense and non-inline BinaryView inputs
  • add focused tests and benchmarks for single-column and mixed BinaryView filter cases related to #9143

Verification

  • cargo test -p arrow-select coalesce --lib
  • cargo clippy -p arrow-select --lib --tests -- -D warnings
  • cargo clippy -p arrow --bench coalesce_kernels --features test_utils -- -D warnings
  • cargo bench -p arrow --bench coalesce_kernels --features test_utils -- --noplot single_binaryview
  • cargo bench -p arrow --bench coalesce_kernels --features test_utils -- --noplot mixed_binaryview

Benchmark Results

Measured against a clean origin/main worktree with the same BinaryView benchmark additions. The figures below compare representative median times from the baseline worktree and this branch.

Mixed primitive + BinaryView

  • mixed_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.001: 23.16 ms -> 8.51 ms
  • mixed_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.01: 2.37 ms -> 1.31 ms
  • mixed_binaryview (max_string_len=8), 8192, nulls: 0.1, selectivity: 0.001: 31.70 ms -> 14.33 ms
  • mixed_binaryview (max_string_len=8), 8192, nulls: 0.1, selectivity: 0.01: 3.92 ms -> 2.44 ms

Single BinaryView

  • single_binaryview, 8192, nulls: 0, selectivity: 0.01: 4.86 ms -> 4.90 ms (roughly flat, slightly slower)
  • single_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.001: 34.72 ms -> 19.33 ms
  • single_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.01: 3.46 ms -> 2.03 ms
  • single_binaryview (max_string_len=8), 8192, nulls: 0.1, selectivity: 0.01: 5.93 ms -> 3.97 ms
  • single_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.8: 597 µs -> 619 µs (regression)
  • single_binaryview (max_string_len=8), 8192, nulls: 0.1, selectivity: 0.8: 1.78 ms -> 1.79 ms (roughly flat, slightly slower)

In short, this change substantially improves the mixed primitive + inline BinaryView path that motivated #9143, while the single-column BinaryView benchmarks still show trade-offs: sparse inline cases improve, but dense inline cases are slightly slower and the non-inline single-column path is effectively unchanged.

Closes #9143.

Avoid materialising an intermediate filtered RecordBatch when coalescing sparse inline BinaryView batches, and reuse shared filter indices for primitive columns in mixed batches.

This addresses issue apache#9143 and adds focused tests and benchmarks for single-column and mixed BinaryView filter paths.

Signed-off-by: cl <cailue@apache.org>
@github-actions github-actions bot added the arrow Changes to the arrow crate label Apr 17, 2026
@ClSlaid
Copy link
Copy Markdown
Contributor Author

ClSlaid commented Apr 17, 2026

/cc @alamb PTAL.

@Dandandan
Copy link
Copy Markdown
Contributor

run benchmark coalesce_kernels

Comment thread arrow-select/src/coalesce/primitive.rs Outdated
Split push_batch_with_filter into an explicit fused path classifier and dedicated inline BinaryView dispatch helpers, and use iterator-based extension in the primitive fast path. This keeps unsupported BinaryView layouts on the existing filter path while making the optimised routing for issue apache#9143 clearer to review and maintain.

Signed-off-by: cl <cailue@apache.org>
@ClSlaid
Copy link
Copy Markdown
Contributor Author

ClSlaid commented Apr 18, 2026

Optimising BinaryView filter will give you a slight overall performance regression on that benchmark. @Dandandan 🫠

Warp up

Following message was gpt produced:

I reran the relevant coalesce_kernels cases on the same machine against both the clean baseline worktree and this patch, so the numbers below are direct baseline-vs-patch comparisons.

Results:

  • single_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.01

    • baseline: 3.3982-3.4646 ms
    • patch: 1.9005-1.9241 ms
    • result: substantial improvement
  • mixed_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.01

    • baseline: 2.3677-2.3976 ms
    • patch: 1.1806-1.1959 ms
    • result: substantial improvement
  • mixed_binaryview (max_string_len=20), 8192, nulls: 0, selectivity: 0.01

    • baseline: 2.8777-2.9365 ms
    • patch: 2.9366-2.9687 ms
    • result: slight regression

I also checked which path the benchmark is actually taking.

  • mixed_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.01
    • routes to the fused direct-copy path
  • single_binaryview (max_string_len=8), 8192, nulls: 0, selectivity: 0.01
    • routes to the fused direct-copy path
  • mixed_binaryview (max_string_len=20), 8192, nulls: 0, selectivity: 0.01
    • stays on the unsupported vanilla path

So the main improvement is in the fully inline BinaryView cases that this patch targets. The remaining regression on mixed_binaryview (max_string_len=20) is not coming from the fused fast path, because that benchmark still does not use the fused path at all: once non-inline BinaryView values are present, it remains on the existing filter_record_batch plus push_batch path.

.as_primitive::<T>();

if let Some(nulls) = s.nulls().filter(|nulls| nulls.null_count() > 0) {
for &idx in indices {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there should be a path based on iterator here as well?

@Dandandan
Copy link
Copy Markdown
Contributor

In my earlier experiments, #8951 I also got good performance on my local machine but was regressing quite a bit on the shared benchmarks based on the @alamb runner (which was running on Intel CPUs).

I got some improvements from increasing the selectivity threshold https://github.com/apache/arrow-rs/pull/8951/files#diff-49658ee26403ad8cc710620108072c017373bea671df8d2a83543b9de13d0e8aL42

and there are some optimizations we can do to the filter iterators to make them faster.

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

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[coalesce] Implement specialized BatchCoalescer::push_batch_with_filter for binaryview array

2 participants