You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by gi...@apache.org on 2026/10/15 05:07:03 UTC
(arrow-rs) branch gh-readonly-queue/main/pr-11528-8576d90f6ea09f03552f24ece149968b312389c2 created (now 94807cd0bf)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-11528-8576d90f6ea09f03552f24ece149968b312389c2
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
at 94807cd0bf feat(arrow-array): add `div_rem_wrapping` and `div_rem_checked` to `ArrowNativeTypeOp` (#11528)
This branch includes the following new commits:
new 94807cd0bf feat(arrow-array): add `div_rem_wrapping` and `div_rem_checked` to `ArrowNativeTypeOp` (#11528)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(arrow-rs) 01/01: feat(arrow-array): add `div_rem_wrapping` and `div_rem_checked` to `ArrowNativeTypeOp` (#11528)
Posted by gi...@apache.org.
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-8576d90f6ea09f03552f24ece149968b312389c2
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 94807cd0bf7390556abb2cbf72cdeb0a7ef2fdcd
Author: Matt Butrovich <mb...@users.noreply.github.com>
AuthorDate: Thu Oct 15 03:58:46 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 {