mbutrovich opened a new pull request, #11425: URL: https://github.com/apache/arrow-rs/pull/11425
# Which issue does this PR close? - Part of #11421. # Rationale for this change `i256` already computes the quotient and remainder together in a private [`div_rem`](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-buffer/src/bigint/mod.rs#L488-L514), but the public API only exposes them separately through [`wrapping_div`, `checked_div`, `wrapping_rem` and `checked_rem`](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-buffer/src/bigint/mod.rs#L535-L573). A caller that needs both runs [`div_rem_knuth`](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-buffer/src/bigint/div.rs#L83) (Knuth's Algorithm D) twice. Comet's [`div_round_half_up`](https://github.com/apache/datafusion-comet/blob/8d0bb01a5a99924024e07c1722e6eddd9260e167/native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs#L123-L146) does this for every row of a wide decimal multiply. # What changes are included in this PR? This PR adds two public methods to `i256`, named after the existing `wrapping_*` and `checked_*` methods. - `wrapping_div_rem(self, other) -> (Self, Self)` returns `(i256::MIN, 0)` for `i256::MIN / -1` and panics on a zero divisor, the same as `wrapping_div` and `wrapping_rem`. - `checked_div_rem(self, other) -> Option<(Self, Self)>` returns `None` for a zero divisor or `i256::MIN / -1`. Both call the private `div_rem`, so its error type stays private. # Are these changes tested? Yes. The tests compare both methods against `num_bigint::BigInt`, which the existing `i256` tests already use as a reference. `test_ops` now checks them, so they run on the candidate pairs in `test_i256` and the random pairs in `test_i256_fuzz`. A new test, `test_div_rem_powers_of_ten`, uses divisors 10^k - 1, 10^k and 10^k + 1 for k from 0 to 76, with dividends at the `i256` and `i128` bounds, around 2^64 and 2^128, and next to the divisor, in every sign combination. Another new test checks that `wrapping_div_rem` panics on a zero divisor. Each method also has a doctest. # Are there any significant user-facing changes? The two methods are new public API. Nothing existing changes. ## AI Disclosure I wrote this PR with help from Claude but understand and support the core approach, implementation, and test coverage. -- 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]
