Support RunEndEncoded arrays in numeric kernels - #11062
yinli-systems wants to merge 3 commits into
Conversation
Signed-off-by: Kevin-Li-2025 <kxl474@student.bham.ac.uk>
Signed-off-by: Kevin-Li-2025 <kxl474@student.bham.ac.uk>
Signed-off-by: Kevin-Li-2025 <kxl474@student.bham.ac.uk>
|
Pushed The regression now checks run ends, physical value length, and nulls explicitly; accepts any NaN only when the flat reference is NaN; and continues comparing every non-NaN value via Validation on the updated head:
This diagnosis, fix, and validation were prepared with Codex assistance. |
|
i think we need some discussion on this first, see my comment: i originally suggested the issue be raised in arrow-rs because i thought we lacked REE support, but given it seems we also lack dictionary support then we need t decide if this is intentional or if we should actually add it |
|
Thanks @Jefffrey; I agree this needs an explicit encoded-type decision before merge. I checked the current layers: direct The narrow capability in this PR is compression-preserving My suggested decision is: accept this narrow REE contract as an opt-in encoded kernel capability, then specify Dictionary and mixed dense/encoded semantics in follow-up work. If maintainers prefer consistent direct-kernel support for both encodings before this lands, I am happy to pause or rescope #11062 after that direction is clear. |
Which issue does this PR close?
Rationale for this change
Numeric kernels currently reject run-end encoded inputs. This prevents consumers such as DataFusion from applying arithmetic to compressed ordering columns without decoding them first.
What changes are included in this PR?
Are these changes tested?
Yes. The new differential tests compare decoded REE results with the existing primitive kernels across all public numeric operations, all run-end integer widths, integer/float/decimal/timestamp values, nulls, checked and wrapping overflow, division errors, unequal run boundaries, every slice offset in a deterministic matrix, empty inputs, and both scalar directions. A billion-row logical array test verifies work and result storage remain proportional to physical runs.
Local validation:
cargo test -p arrow-arith --all-features: 239 unit tests, 13 integration tests, and 16 doctests passed.cargo test --lib --tests --release -p arrow-arith: 239 unit tests and 13 integration tests passed.cargo test -p arrow --features=force_validate,prettyprint,ipc_compression,ffi,chrono-tz: passed.cargo clippy -p arrow-arith --all-targets --all-features -- -D warnings: passed.cargo +stable fmt --all -- --check: passed.cargo bench -p arrow-arith --bench run_arithmetic -- --quick: passed.The committed Criterion benchmark compares scalar, aligned-boundary, and staggered-boundary REE arithmetic with arithmetic over already-resident dense arrays at one million logical rows. On this machine, runs of 128 rows were 2.5x-13.7x faster and runs of 4096 rows were 68x-295x faster. Runs of 8 rows were 13%-6.5x slower, so this PR does not claim REE is beneficial for low-compression data. The billion-row test covers the non-expansion property deterministically.
The same integration test file was also run against the unmodified base: 12 of 13 tests failed because REE numeric operations were unsupported; only the invalid-input test passed.
Are there any user-facing changes?
Yes. Numeric arithmetic and negation can now operate directly on REE arrays when the other operand is a scalar or REE array. The result remains run-end encoded. There are no public API signature changes.
AI disclosure: Codex assisted with repository research, implementation, test generation, performance analysis, and self-review. I reviewed the resulting behavior through differential tests, negative controls against the base revision, release-mode tests, strict Clippy, and the compression benchmark. No independent human review is claimed.