You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "sjhddh (via GitHub)" <gi...@apache.org> on 2026/06/03 18:46:31 UTC
[PR] fix(arrow-arith): avoid intermediate overflow in Decimal256 division [arrow-rs]
sjhddh opened a new pull request, #10059:
URL: https://github.com/apache/arrow-rs/pull/10059
# Which issue does this PR close?
- Closes #7216.
# Rationale for this change
Dividing two `Decimal256` values whose result is representable can still fail with
`ArithmeticOverflow`. The `Op::Div` path pre-scales the numerator by `10^mul_pow`
(to add the +4 fractional digits Hive/postgres semantics require) before dividing.
For e.g. `Decimal256(38, 37) / Decimal256(38, 37)` this intermediate `l * 10^41`
exceeds `i256::MAX` even though the true quotient (~1.0) easily fits, so a valid
division spuriously errors.
# What changes are included in this PR?
- Adds `decimal_div_scaled`, which computes `trunc(l * 10^pow / r)`:
- Fast path: the existing `(l * 10^pow) / r` formula when it doesn't overflow
(common case, behavior unchanged).
- Slow path (only on overflow): schoolbook base-10 long division on magnitudes that
brings down one digit at a time and computes each digit via repeated modular
addition, so no intermediate value ever exceeds the divisor. This avoids the
intermediate overflow while preserving truncate-toward-zero semantics, and is sound
even when the divisor is close to `i256::MAX` (where `remainder * 10` itself would
overflow).
- Rewires the `Op::Div` arm to use the helper for the scale-up case; the
divisor-only-scaled and no-scaling cases keep the original direct formula.
# Are these changes tested?
Yes. Added `test_decimal256_div` (the repro from the issue) plus assertions for all four
sign combinations and a near-`i256::MAX` divisor edge case. The test overflows on `main`
and passes with this change; existing decimal add/sub/mul/div/rem/divide-by-zero tests
still pass (`cargo test -p arrow-arith`, clippy `-D warnings`, fmt all clean).
# Are there any user-facing changes?
No API changes. `Decimal128`/`Decimal256` division that previously returned a spurious
`ArithmeticOverflow` now returns the correct quotient. Results that genuinely don't fit
the native type still return `ArithmeticOverflow`, and divide-by-zero still returns
`DivideByZero`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arrow-arith): avoid intermediate overflow in Decimal256 division [arrow-rs]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10059:
URL: https://github.com/apache/arrow-rs/pull/10059#issuecomment-5235180956
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arrow-arith): avoid intermediate overflow in Decimal256 division [arrow-rs]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10059: fix(arrow-arith): avoid intermediate overflow in Decimal256 division
URL: https://github.com/apache/arrow-rs/pull/10059
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org