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-11528-3983bed2f9b6b1a85514d15d56c358622b29468c in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit c80ada4937f614b5282e038c07e2d7b0203a645a Author: Matt Butrovich <[email protected]> AuthorDate: Wed Oct 14 19:57:31 2026 +0000 feat(arrow-array): add `div_rem_wrapping` and `div_rem_checked` to `ArrowNativeTypeOp` (#11528) # Which issue does this PR close? - Closes #11454. - Part of #11421. # Rationale for this change See #11454. # What changes are included in this PR? `ArrowNativeTypeOp` gains `div_rem_wrapping` and `div_rem_checked`, which return `(quotient, remainder)`. Their default implementations call `div_*` and `mod_*`. `i256` overrides them with `i256::wrapping_div_rem` and `i256::checked_div_rem` from #11425, and `div_rem_checked` returns the same errors as `div_checked`. `native_type_op!` takes optional trailing items so the `i256` impl can carry the overrides. The names follow the `*_wrapping` and `*_checked` suffixes of the other `ArrowNativeTypeOp` methods. `checked_div_rem` would share a name with `i256::checked_div_rem` but return `Result` instead of `Option`. `make_downscaler` and `scaled_div` call the new methods. `ArrowNativeType` is sealed, so adding trait methods with defaults doesn't break downstream code. # Are these changes tested? `test_i256_div_rem` and `test_i256_div_rem_wrapping_zero_panics` in `arrow-array/src/arithmetic.rs` check the `i256` overrides against `div_*` and `mod_*`, including the errors. The existing tests cover both callers with `Decimal256` and negative values, such as `test_cast_decimal_to_decimal_round` and `test_decimal256_div_wide_intermediate`. The benchmark bot ran the `Decimal256` cases of `decimal_downscale` in the `cast_decimal` benchmark from #11426 on [x86_64](https://github.com/apache/arrow-rs/pull/11528#issuecomment-6131548323) and [arm64](https://github.com/apache/arrow-rs/pull/11528#issuecomment-6131541220), and `decimal256_scalar/div_72_digits_by_4_digits` in the `decimal_division` benchmark from #11427, which reaches `scaled_div`, on [x86_64](https://github.com/apache/arrow-rs/pull/11528#issuecomment-6131473177) and [arm64](https://github.com/apache/arrow-rs/pull/11528#issuecomment-6131472782). The runners are a Xeon Platinum 8581C and a Neoverse-V2. <img width="1600" height="1124" alt="div-rem-bot" src="https://github.com/user-attachments/assets/e2d02705-8357-47b0-b473-447b8910eee2"; /> <details><summary>Numbers behind the chart</summary> In us with no nulls, per 8192 values for the casts and per 1024 values for the division: | Case | x86_64: `main` | This PR | arm64: `main` | This PR | |---|---|---|---|---| | `Decimal256` (76, 10) to (76, 2), 38 digits | 260.7 | 172.0 | 467.7 | 265.5 | | `Decimal256` (76, 10) to (76, 2), 60 digits | 261.1 | 167.4 | 751.5 | 422.4 | | `Decimal256` (41, 20) to `Decimal128` (38, 6), 30 digits | 278.8 | 174.6 | 499.9 | 291.3 | | `Decimal256` (76, 20) to `Decimal128` (38, 6), 48 digits, fallible | 280.7 | 211.1 | 780.7 | 443.1 | | `Decimal256` (76, 40) to `Decimal128` (38, 6), 42 digits, fallible | 582.3 | 340.9 | 702.5 | 390.4 | | `Decimal256` 72 digits by 4 digits | 405.8 | 380.7 | 550.3 | 508.5 | </details> The cases with nulls and the strict casts changed by about the same amount. Most of the division case's time goes to the per-digit loop in `scaled_div` and to the error that #11479 stops building. <details><summary>Apple M5 Max</summary> With rustc 1.99.0, in us per 8192 values with no nulls: | Case | `main` | This PR | |---|---|---| | `Decimal256` (76, 10) to (76, 2), 38 digits | 178.9 | 104.4 | | `Decimal256` (76, 10) to (76, 2), 60 digits | 272.1 | 154.0 | | `Decimal256` (41, 20) to `Decimal128` (38, 6), 30 digits | 263.0 | 153.4 | | `Decimal256` (76, 20) to `Decimal128` (38, 6), 48 digits, fallible | 460.4 | 294.9 | | `Decimal256` (76, 40) to `Decimal128` (38, 6), 42 digits, fallible | 380.5 | 227.7 | `Decimal128` keeps the default methods, and its cases changed by 3% or less, within the difference between repeated runs. The division case went from 295 to 284 us per 1024 values, the mean of two alternating runs each. </details> # Are there any significant user-facing changes? `ArrowNativeTypeOp` has two new methods with default implementations. ## AI Disclosure I wrote this PR with help from Claude but understand and support the core approach, implementation, and test coverage. --- arrow-arith/src/numeric.rs | 3 +- arrow-array/src/arithmetic.rs | 109 +++++++++++++++++++++++++++++++++++++++-- arrow-cast/src/cast/decimal.rs | 3 +- 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/arrow-arith/src/numeric.rs b/arrow-arith/src/numeric.rs index ea2a51a6e4..acaa886d31 100644 --- a/arrow-arith/src/numeric.rs +++ b/arrow-arith/src/numeric.rs @@ -982,8 +982,7 @@ fn scaled_div<T: DecimalType>( let dividend = abs_checked::<T>(l)?; let divisor = abs_checked::<T>(r)?; - let mut quotient = dividend.div_checked(divisor)?; - let mut remainder = dividend.mod_checked(divisor)?; + let (mut quotient, mut remainder) = dividend.div_rem_checked(divisor)?; for _ in 0..mul_pow { // `remainder * 10` overflows for divisors near `T::Native::MAX`, so add the remainder // ten times and take the divisor off whenever the running sum reaches it. diff --git a/arrow-array/src/arithmetic.rs b/arrow-array/src/arithmetic.rs index ea43ed320e..eccb91cda8 100644 --- a/arrow-array/src/arithmetic.rs +++ b/arrow-array/src/arithmetic.rs @@ -28,8 +28,9 @@ use std::cmp::Ordering; /// types they will wrap around the boundary of the type. For floating point types they /// will overflow to INF or -INF preserving the expected sign value /// -/// Note `div_wrapping` and `mod_wrapping` will panic for integer types if `rhs` is zero -/// although this may be subject to change <https://github.com/apache/arrow-rs/issues/2647> +/// Note `div_wrapping`, `mod_wrapping` and `div_rem_wrapping` will panic for integer types if +/// `rhs` is zero, although this may be subject to change +/// <https://github.com/apache/arrow-rs/issues/2647> /// /// The APIs with `_checked` suffix perform overflow-checking. For integer types /// these will return `Err` instead of wrapping. For floating point types they will @@ -93,6 +94,43 @@ pub trait ArrowNativeTypeOp: ArrowNativeType { /// Panics if `rhs` is zero for integer types fn mod_wrapping(self, rhs: Self) -> Self; + /// Checked division and remainder operation, returning `(quotient, remainder)` + /// + /// Returns the same results as [`Self::div_checked`] and [`Self::mod_checked`]. `i256` + /// computes both with one division. + /// + /// # Example + /// ``` + /// # use arrow_array::ArrowNativeTypeOp; + /// assert_eq!((-7_i32).div_rem_checked(2).unwrap(), (-3, -1)); + /// assert!(7_i32.div_rem_checked(0).is_err()); + /// assert!(i32::MIN.div_rem_checked(-1).is_err()); + /// ``` + #[inline] + fn div_rem_checked(self, rhs: Self) -> Result<(Self, Self), ArrowError> { + Ok((self.div_checked(rhs)?, self.mod_checked(rhs)?)) + } + + /// Wrapping division and remainder operation, returning `(quotient, remainder)` + /// + /// Returns the same results as [`Self::div_wrapping`] and [`Self::mod_wrapping`]. `i256` + /// computes both with one division. + /// + /// # Panics + /// + /// Panics if `rhs` is zero for integer types + /// + /// # Example + /// ``` + /// # use arrow_array::ArrowNativeTypeOp; + /// assert_eq!((-7_i32).div_rem_wrapping(2), (-3, -1)); + /// assert_eq!(i32::MIN.div_rem_wrapping(-1), (i32::MIN, 0)); + /// ``` + #[inline] + fn div_rem_wrapping(self, rhs: Self) -> (Self, Self) { + (self.div_wrapping(rhs), self.mod_wrapping(rhs)) + } + /// Checked negation operation fn neg_checked(self) -> Result<Self, ArrowError>; @@ -152,8 +190,10 @@ macro_rules! native_type_op { ($t:tt, $zero:expr, $one: expr) => { native_type_op!($t, $zero, $one, $t::MIN, $t::MAX); }; - ($t:tt, $zero:expr, $one: expr, $min: expr, $max: expr) => { + ($t:tt, $zero:expr, $one: expr, $min: expr, $max: expr $(, $extra:item)*) => { impl ArrowNativeTypeOp for $t { + $($extra)* + const ZERO: Self = $zero; const ONE: Self = $one; const MIN_TOTAL_ORDER: Self = $min; @@ -296,7 +336,26 @@ native_type_op!(u8); native_type_op!(u16); native_type_op!(u32); native_type_op!(u64); -native_type_op!(i256, i256::ZERO, i256::ONE); +native_type_op!( + i256, + i256::ZERO, + i256::ONE, + i256::MIN, + i256::MAX, + #[inline] + fn div_rem_checked(self, rhs: Self) -> Result<(Self, Self), ArrowError> { + if rhs.is_zero() { + return Err(ArrowError::DivideByZero); + } + self.checked_div_rem(rhs).ok_or_else(|| { + ArrowError::ArithmeticOverflow(format!("Overflow happened on: {self:?} / {rhs:?}")) + }) + }, + #[inline] + fn div_rem_wrapping(self, rhs: Self) -> (Self, Self) { + self.wrapping_div_rem(rhs) + } +); native_type_op!(IntervalDayTime, IntervalDayTime::ZERO, IntervalDayTime::ONE); native_type_op!( @@ -777,6 +836,48 @@ mod tests { assert_eq!(9.0_f64.mod_checked(2.0_f64).unwrap(), 1_f64); } + #[test] + fn test_i256_div_rem() { + let values = [ + i256::MIN, + i256::MIN + i256::ONE, + i256::from_i128(-7), + i256::MINUS_ONE, + i256::ZERO, + i256::ONE, + i256::from_i128(2), + i256::from_i128(7), + i256::from_i128(i128::MAX), + i256::MAX, + ]; + for n in values { + for d in values { + let checked = n + .div_checked(d) + .and_then(|q| Ok((q, n.mod_checked(d)?))) + .map_err(|e| e.to_string()); + assert_eq!( + n.div_rem_checked(d).map_err(|e| e.to_string()), + checked, + "{n} / {d}" + ); + if !d.is_zero() { + assert_eq!( + n.div_rem_wrapping(d), + (n.div_wrapping(d), n.mod_wrapping(d)), + "{n} / {d}" + ); + } + } + } + } + + #[test] + #[should_panic(expected = "attempt to divide by zero")] + fn test_i256_div_rem_wrapping_zero_panics() { + let _ = i256::ONE.div_rem_wrapping(i256::ZERO); + } + #[test] #[cfg_attr(miri, ignore)] // Unsupported inline assembly fn test_native_type_neg() { diff --git a/arrow-cast/src/cast/decimal.rs b/arrow-cast/src/cast/decimal.rs index cfc8e3e624..2a11a142a6 100644 --- a/arrow-cast/src/cast/decimal.rs +++ b/arrow-cast/src/cast/decimal.rs @@ -237,8 +237,7 @@ where let f_fallible = move |x: I::Native| { // div is >= 10 and so this cannot overflow - let d = x.div_wrapping(div); - let r = x.mod_wrapping(div); + let (d, r) = x.div_rem_wrapping(div); // Round result let adjusted = match x >= I::Native::ZERO {
