This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11425-92be67a7f8cce3ee87f4234def067bb067cfd9a1 in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 66dd257bc25f908ae4550fa080899fd194d0e052 Author: Matt Butrovich <[email protected]> AuthorDate: Thu Oct 8 17:47:49 2026 +0000 feat(arrow-buffer): add `i256::wrapping_div_rem` and `i256::checked_div_rem` (#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. --- arrow-buffer/src/bigint/mod.rs | 182 ++++++++++++++++++++++++++++++----------- 1 file changed, 136 insertions(+), 46 deletions(-) diff --git a/arrow-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs index db996acfdb..21f7664207 100644 --- a/arrow-buffer/src/bigint/mod.rs +++ b/arrow-buffer/src/bigint/mod.rs @@ -534,16 +534,14 @@ impl i256 { /// Performs wrapping division /// + /// Use [`Self::wrapping_div_rem`] if you also need the remainder. + /// /// # Panics /// /// Panics if `other` is zero #[inline] pub fn wrapping_div(self, other: Self) -> Self { - match self.div_rem(other) { - Ok((v, _)) => v, - Err(DivRemError::DivideByZero) => panic!("attempt to divide by zero"), - Err(_) => Self::MIN, - } + self.wrapping_div_rem(other).0 } /// Performs checked division @@ -554,16 +552,14 @@ impl i256 { /// Performs wrapping remainder /// + /// Use [`Self::wrapping_div_rem`] if you also need the quotient. + /// /// # Panics /// /// Panics if `other` is zero #[inline] pub fn wrapping_rem(self, other: Self) -> Self { - match self.div_rem(other) { - Ok((_, v)) => v, - Err(DivRemError::DivideByZero) => panic!("attempt to divide by zero"), - Err(_) => Self::ZERO, - } + self.wrapping_div_rem(other).1 } /// Performs checked remainder @@ -572,6 +568,61 @@ impl i256 { self.div_rem(other).map(|(_, v)| v).ok() } + /// Performs wrapping division and remainder, returning `(quotient, remainder)` + /// + /// This computes both results with one division, while calling + /// [`Self::wrapping_div`] and [`Self::wrapping_rem`] divides twice. + /// + /// The quotient is truncated toward zero, and the remainder has the sign of + /// `self`. `i256::MIN / -1` wraps to `(i256::MIN, 0)`. + /// + /// # Panics + /// + /// Panics if `other` is zero + /// + /// # Example + /// ``` + /// # use arrow_buffer::i256; + /// let (q, r) = i256::from_i128(-7).wrapping_div_rem(i256::from_i128(2)); + /// assert_eq!((q, r), (i256::from_i128(-3), i256::from_i128(-1))); + /// + /// let (q, r) = i256::MIN.wrapping_div_rem(i256::MINUS_ONE); + /// assert_eq!((q, r), (i256::MIN, i256::ZERO)); + /// ``` + #[inline] + pub fn wrapping_div_rem(self, other: Self) -> (Self, Self) { + match self.div_rem(other) { + Ok(v) => v, + Err(DivRemError::DivideByZero) => panic!("attempt to divide by zero"), + Err(DivRemError::DivideOverflow) => (Self::MIN, Self::ZERO), + } + } + + /// Performs checked division and remainder, returning `(quotient, remainder)` + /// + /// This computes both results with one division, while calling + /// [`Self::checked_div`] and [`Self::checked_rem`] divides twice. + /// + /// The quotient is truncated toward zero, and the remainder has the sign of + /// `self`. + /// + /// Returns `None` if `other` is zero or the quotient overflows, which happens + /// only for `i256::MIN / -1` + /// + /// # Example + /// ``` + /// # use arrow_buffer::i256; + /// let qr = i256::from_i128(-7).checked_div_rem(i256::from_i128(2)); + /// assert_eq!(qr, Some((i256::from_i128(-3), i256::from_i128(-1)))); + /// + /// assert_eq!(i256::ONE.checked_div_rem(i256::ZERO), None); + /// assert_eq!(i256::MIN.checked_div_rem(i256::MINUS_ONE), None); + /// ``` + #[inline] + pub fn checked_div_rem(self, other: Self) -> Option<(Self, Self)> { + self.div_rem(other).ok() + } + /// Performs checked exponentiation #[inline] pub const fn checked_pow(self, mut exp: u32) -> Option<Self> { @@ -1454,42 +1505,8 @@ mod tests { ), } - // Division - if ir != i256::ZERO { - let actual = il.wrapping_div(ir); - let expected = bl.clone() / br.clone(); - let checked = il.checked_div(ir); - - if ir == i256::MINUS_ONE && il == i256::MIN { - // BigInt produces an integer over i256::MAX - assert_eq!(actual, i256::MIN); - assert!(checked.is_none()); - } else { - assert_eq!(actual.to_string(), expected.to_string()); - assert_eq!(checked.unwrap().to_string(), expected.to_string()); - } - } else { - // `wrapping_div` panics on division by zero - assert!(il.checked_div(ir).is_none()); - } - - // Remainder - if ir != i256::ZERO { - let actual = il.wrapping_rem(ir); - let expected = bl.clone() % br.clone(); - let checked = il.checked_rem(ir); - - assert_eq!(actual.to_string(), expected.to_string(), "{il} % {ir}"); - - if ir == i256::MINUS_ONE && il == i256::MIN { - assert!(checked.is_none()); - } else { - assert_eq!(checked.unwrap().to_string(), expected.to_string()); - } - } else { - // `wrapping_rem` panics on division by zero - assert!(il.checked_rem(ir).is_none()); - } + // Division and remainder + test_div_rem(il, ir); // Exponentiation for exp in [0, 1, 2, 3, 8, 100] { @@ -1621,6 +1638,79 @@ mod tests { } } + fn test_div_rem(n: i256, d: i256) { + if d == i256::ZERO { + // The wrapping methods panic on division by zero + assert_eq!(n.checked_div(d), None); + assert_eq!(n.checked_rem(d), None); + assert_eq!(n.checked_div_rem(d), None); + return; + } + + let bn = BigInt::from_signed_bytes_le(&n.to_le_bytes()); + let bd = BigInt::from_signed_bytes_le(&d.to_le_bytes()); + + // BigInt produces an integer over i256::MAX for i256::MIN / -1 + let (q, overflow) = i256::from_bigint_with_overflow(bn.clone() / bd.clone()); + let (r, _) = i256::from_bigint_with_overflow(bn % bd); + + assert_eq!(n.wrapping_div(d), q, "{n} / {d}"); + assert_eq!(n.wrapping_rem(d), r, "{n} % {d}"); + assert_eq!(n.wrapping_div_rem(d), (q, r), "{n} / {d}"); + assert_eq!(n.checked_div(d), (!overflow).then_some(q), "{n} / {d}"); + assert_eq!(n.checked_rem(d), (!overflow).then_some(r), "{n} % {d}"); + assert_eq!( + n.checked_div_rem(d), + (!overflow).then_some((q, r)), + "{n} / {d}" + ); + } + + #[test] + #[cfg_attr(miri, ignore)] // Takes too long + fn test_div_rem_powers_of_ten() { + let dividends = [ + i256::ZERO, + i256::ONE, + i256::MIN, + i256::MAX, + i256::from_i128(i128::MIN), + i256::from_i128(i128::MAX), + i256::from_i128(u64::MAX as i128), + i256::from_i128(1 << 64), + i256::from_i128((1 << 64) + 1), + i256::from_parts(u128::MAX, 0), + i256::from_parts(0, 1), + i256::from_parts(1, 1), + ]; + + // 10^76 is the largest power of ten that fits in an i256 + for k in 0..=76 { + let p = i256::from_i128(10).wrapping_pow(k); + for d in [p - i256::ONE, p, p + i256::ONE] { + for n in dividends + .into_iter() + .chain([p - i256::ONE, p, p + i256::ONE]) + { + for (n, d) in [ + (n, d), + (n.wrapping_neg(), d), + (n, -d), + (n.wrapping_neg(), -d), + ] { + test_div_rem(n, d); + } + } + } + } + } + + #[test] + #[should_panic(expected = "attempt to divide by zero")] + fn test_wrapping_div_rem_zero_panics() { + let _ = i256::ONE.wrapping_div_rem(i256::ZERO); + } + #[test] fn test_signed_ops() { // signum
