Dandandan opened a new pull request, #23203:
URL: https://github.com/apache/datafusion/pull/23203
## Which issue does this PR close?
<!-- No issue filed; small self-contained perf change. Happy to open one if
preferred. -->
- Closes #.
## Rationale for this change
`ScalarValue::add` / `add_checked` already short-circuit the common
primitive and decimal cases through an in-place `try_add_*_in_place` path,
avoiding the `to_scalar()` → Arrow kernel → `try_from_array` round-trip (≈3
heap allocations + a kernel dispatch per call). `sub` / `sub_checked` did
**not** — they always took the slow kernel path.
This matters for window functions:
`WindowFrameStateRange::calculate_index_of_row` calls `ScalarValue::sub` /
`sub_checked` **once per row** to compute RANGE frame boundaries for
PRECEDING-on-ascending / FOLLOWING-on-descending offsets, so every row in such
a partition paid the allocation + dispatch cost. This brings `sub` to parity
with the already-optimized `add`.
## What changes are included in this PR?
- Add `sub_optional`, `sub_decimal_values`, and
`try_sub_{wrapping,checked}_in_place`, mirroring the existing `add` helpers.
- Route `sub` / `sub_checked` through the in-place fast path, gated by the
whitelist (renamed `can_use_direct_add` → `can_use_direct_arithmetic` since it
now serves both ops), with the Arrow kernel path retained as the fallback for
non-whitelisted types.
- The decimal path stays **always-checked** (`mul_checked` + `sub_checked`)
to match the Arrow decimal kernels exactly, regardless of the wrapping vs.
checked variant.
No public API or behavior change — `sub`/`sub_checked` produce identical
results (and identical overflow errors) to the kernel path.
## Are these changes tested?
Yes. Existing `sub` tests (including the exact overflow error messages)
continue to pass — the fast path uses the same `ArrowNativeTypeOp::sub_checked`
the Arrow kernel calls internally, so messages are unchanged. Added parity
tests covering:
- primitive wrapping subtraction incl. signed wrap-around and unsigned
underflow,
- checked overflow/underflow (errors) and the no-overflow case,
- NULL propagation on either operand,
- decimal scale reconciliation for Decimal128/Decimal256.
## Are there any user-facing changes?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
