Skip to content

Support RunEndEncoded arrays in numeric kernels - #11062

Open
yinli-systems wants to merge 3 commits into
apache:mainfrom
yinli-systems:kevin/ree-arithmetic-10891
Open

yinli-systems wants to merge 3 commits into
apache:mainfrom
yinli-systems:kevin/ree-arithmetic-10891

Conversation

@yinli-systems

@yinli-systems yinli-systems commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

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?

  • Route the checked, wrapping, division, remainder, and negation numeric entry points through a shared REE implementation.
  • Preserve run boundaries for scalar operations and use the union of boundaries for two REE operands. Aligned inputs avoid gathering their physical values.
  • Normalize sliced run ends, preserve primitive null/overflow/decimal/temporal semantics, and select the wider run-end integer type for two differently encoded operands.
  • Return an explicit error for a REE array combined with a non-scalar unencoded array. That case would require expansion to produce a dense result and is outside this compression-preserving path. Direct arrow-arith numeric kernels also do not currently accept Dictionary arrays; encoded-type policy is under discussion.

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.

Signed-off-by: Kevin-Li-2025 <kxl474@student.bham.ac.uk>
@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-arith labels Sep 11, 2026
Signed-off-by: Kevin-Li-2025 <kxl474@student.bham.ac.uk>
Signed-off-by: Kevin-Li-2025 <kxl474@student.bham.ac.uk>
@yinli-systems

Copy link
Copy Markdown
Contributor Author

Pushed 5143d92fe to address the MIRI (5) failure in run_float_ieee_semantics. The two arrays differed only in the sign bit of NaN (+qNaN versus -qNaN), which IEEE arithmetic does not guarantee to preserve across equivalent evaluation paths.

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 to_bits(), preserving exact signed-zero and infinity behavior.

Validation on the updated head:

  • matching 2026-09-11 nightly Miri: focused regression passes
  • cargo test -p arrow-arith: 239 unit tests, 13 integration tests, and 16 doctests pass
  • strict package Clippy with all targets/features passes
  • stable rustfmt and git diff --check pass

This diagnosis, fix, and validation were prepared with Codex assistance.

@Jefffrey

Copy link
Copy Markdown
Contributor

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

@yinli-systems

Copy link
Copy Markdown
Contributor Author

Thanks @Jefffrey; I agree this needs an explicit encoded-type decision before merge. I checked the current layers: direct arrow_arith::numeric::{add,sub,mul,div,neg,...} dispatch accepts primitives but not Dictionary; DataFusion already has dictionary arithmetic tests such as plus_op_dict, with its coercion/evaluation layer producing dense primitive results. The DataFusion #24565 change is about REE coercion and window range extraction, not evidence that direct Arrow numeric kernels support dictionaries. Conversely, arrow_arith::temporal::date_part handles both Dictionary and REE by transforming physical values, so there does not appear to be a universal policy forbidding encoded inputs in Arrow kernels.

The narrow capability in this PR is compression-preserving REE x scalar and REE x REE arithmetic, where the output remains REE and work scales with runs. REE x dense non-scalar is rejected rather than silently expanding to logical length. Dictionary arithmetic has different cardinality/key-pair questions, so I would treat its direct-kernel contract as a separate design item instead of adding an unreviewed implementation here. I corrected the PR description, which had implied a stronger dictionary precedent than actually exists.

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.

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 arrow-arith

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support RunEndEncoded arrays in arrow-arith numeric kernels (add/sub/mul/div)

2 participants