bharadwaj-pendyala opened a new pull request, #10911: URL: https://github.com/apache/arrow-rs/pull/10911
# Which issue does this PR close? - Closes #7216. # Rationale for this change `Op::Div` scales the left operand before dividing: `result_scale = s1 + 4`, then `mul_pow = result_scale - s1 + s2`. For large operand scales, `mul_pow` includes all of `s2`, so `l * 10^mul_pow` overflows even when the quotient fits. On `main` at 4962d38, the issue example is `Decimal256(38, 37) / Decimal256(38, 37)` with `l = 60096743305738933273387748827369321010`, `r = 60096763826458053191384497987259478584`, and `mul_pow = 41`. `l * 10^41 = 6.0e78`, above `i256::MAX = 5.8e76`, while the quotient is `9.9999965853869970143724273117679321341339e40` at scale 41. Today this reports `Arithmetic overflow: Overflow happened on: 60096743305738933273387748827369321010 * 100000000000000000000000000000000000000000`, despite 35 digits of result headroom. # What changes are included in this PR? `scaled_div` computes `l * 10^mul_pow / r` digit by digit after `mul_checked` overflows; the fast path is unchanged. It carries the remainder forward without calculating `remainder * 10`, adding it ten times and subtracting the divisor when needed. This also handles divisors above `T::Native::MAX / 10`, where that multiply would overflow. The calculation uses magnitudes and restores the sign, preserving truncation toward zero for all four sign combinations. # Are these changes tested? Four tests in `arrow-arith/src/numeric.rs` cover the issue operands, each negated in turn, and a zero divisor; a `Decimal256(76, 37)` divisor of `6e75` above `i256::MAX / 10`; the same wide-intermediate case for `Decimal128`; and `scaled_div` against `i128` `l * 10^mul_pow / r` across both signs, a zero numerator, and operands near `i128::MAX / 10`. I also checked 1000 random `Decimal256(76, 70)` pairs against Python integer division: 661 now return exact values with no sign or truncation disagreements. The remaining 339 have quotients that do not fit `i256`; that harness is not in the diff. `cargo test -p arrow-arith` gives 239 passed / 0 failed, `cargo test -p arrow` is clean, and `cargo fmt --all --check` and `cargo clippy -p arrow-arith --all-targets -- -D warnings` are quiet. # Are there any user-facing changes? No API change. A zero divisor whose numerator would have overflowed now reports `Divide by zero error` instead of the multiplication overflow, because the zero check no longer sits behind scaling. Unrepresentable divisions still error, but the message names the multiplication the digit loop failed on rather than `l * 10^mul_pow`. `T::Native::MIN` remains unsupported as either operand because taking its magnitude overflows. No array with a valid precision can hold it: `MIN` has more digits than `MAX_PRECISION` allows for every decimal type. -- 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]
