This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 648ae60c38 Avoid overflow in decimal precision and scale arithmetic
(#10996)
648ae60c38 is described below
commit 648ae60c38e7275ad165d82b591621913e0c6158
Author: Geonho <[email protected]>
AuthorDate: Wed Sep 30 17:43:40 2026 +0900
Avoid overflow in decimal precision and scale arithmetic (#10996)
# Which issue does this PR close?
Closes #10978.
# Rationale for this change
Valid negative decimal scales can overflow `i8` precision/scale
calculations, causing debug panics or incorrect metadata and cast
fast-path selection when overflow checks are disabled.
# What changes are included in this PR?
- Compute add/subtract/remainder precision and scale differences in
`i16`, clamping precision before narrowing.
- Reject unrepresentable multiplication scales and overflowing remainder
alignment factors.
- Widen decimal cast scale differences and precision checks while
preserving existing rounding, nulls, and safe/unsafe cast behavior.
# Are these changes tested?
Regression tests cover negative-scale metadata, adjacent minimum scales,
alignment overflow, multiplication scale limits, and safe/unsafe decimal
casts. Arithmetic and cast cases are split by expected metadata and
failure path, with exact error assertions. The original failure
reproduction is recorded against base `b1aa94b0`.
- `cargo +stable test -p arrow-arith -p arrow-cast --lib`: 628 passed.
- `cargo +stable --config
'profile.test.package.arrow-arith.overflow-checks=false' --config
'profile.test.package.arrow-cast.overflow-checks=false' test -p
arrow-arith -p arrow-cast --lib decimal256 --verbose`: 36 passed. This
is an unchecked test profile, not an optimized release build.
- `cargo +stable fmt --all -- --check` and `git diff --check`: passed.
- `cargo +stable clippy -p arrow-arith -p arrow-cast --lib --tests
--no-deps -- -D warnings -A clippy::chunks_exact_to_as_chunks`: passed.
Local checks used Rust 1.98.0 on Windows because the installed
repository-pinned 1.97.1 toolchain was incomplete. Strict Clippy first
reported the new `chunks_exact_to_as_chunks` lint in unchanged
`arrow-buffer` and `arrow-arith/src/aggregate.rs` code. The scoped run
allows only that lint; the repository's existing removed-lint
configuration also emits a compatibility warning. No source or lint
configuration was changed for these diagnostics.
# Are there any user-facing changes?
Affected addition, subtraction, multiplication, remainder, and decimal
casts now return correctly calculated results or checked errors for
extreme negative scales instead of panicking or wrapping metadata.
Public API signatures are unchanged.
AI assistance: OpenAI Codex generated the implementation, regression
tests, review follow-up, and PR text, and ran the checks above. The
author personally reviewed the original change at `66680fd2`; the
follow-up refactoring and test revisions were reviewed by Codex agents.
---
arrow-arith/src/numeric.rs | 142 +++++++++++++++++++++++++++++++++++++----
arrow-cast/src/cast/decimal.rs | 28 ++++++--
arrow-cast/src/cast/mod.rs | 121 +++++++++++++++++++++++++++++++++++
3 files changed, 274 insertions(+), 17 deletions(-)
diff --git a/arrow-arith/src/numeric.rs b/arrow-arith/src/numeric.rs
index f3a87a74d3..170d013d6d 100644
--- a/arrow-arith/src/numeric.rs
+++ b/arrow-arith/src/numeric.rs
@@ -1042,14 +1042,13 @@ fn decimal_op<T: DecimalType>(
//
https://cwiki.apache.org/confluence/download/attachments/27362075/Hive_Decimal_Precision_Scale_Support.pdf
let array: PrimitiveArray<T> = match op {
Op::Add | Op::AddWrapping | Op::Sub | Op::SubWrapping => {
+ let (p1, s1, p2, s2) = (*p1 as i16, *s1 as i16, *p2 as i16, *s2 as
i16);
// max(s1, s2)
- let result_scale = *s1.max(s2);
+ let result_scale = s1.max(s2);
// max(s1, s2) + max(p1-s1, p2-s2) + 1
let result_precision =
- (result_scale.saturating_add((*p1 as i8 - s1).max(*p2 as i8 -
s2)) as u8)
- .saturating_add(1)
- .min(T::MAX_PRECISION);
+ (result_scale + (p1 - s1).max(p2 - s2) +
1).min(T::MAX_PRECISION as i16) as u8;
let l_mul = T::Native::usize_as(10).pow_checked((result_scale -
s1) as _)?;
let r_mul = T::Native::usize_as(10).pow_checked((result_scale -
s2) as _)?;
@@ -1082,12 +1081,12 @@ fn decimal_op<T: DecimalType>(
}
_ => unreachable!(),
}
- .with_precision_and_scale(result_precision, result_scale)?
+ .with_precision_and_scale(result_precision, result_scale as i8)?
}
Op::Mul | Op::MulWrapping => {
let result_precision = p1.saturating_add(p2 +
1).min(T::MAX_PRECISION);
- let result_scale = s1.saturating_add(*s2);
- if result_scale > T::MAX_SCALE {
+ let result_scale = *s1 as i16 + *s2 as i16;
+ if result_scale > T::MAX_SCALE as i16 {
// SQL standard says that if the resulting scale of a multiply
operation goes
// beyond the maximum, rounding is not acceptable and thus an
error occurs
return Err(ArrowError::InvalidArgumentError(format!(
@@ -1097,6 +1096,14 @@ fn decimal_op<T: DecimalType>(
T::MAX_SCALE
)));
}
+ let result_scale = i8::try_from(result_scale).map_err(|_| {
+ ArrowError::InvalidArgumentError(format!(
+ "Output scale of {} {op} {} would be less than min scale
of {}",
+ l.data_type(),
+ r.data_type(),
+ i8::MIN
+ ))
+ })?;
try_op!(l, l_s, r, r_s, l.mul_checked(r))
.with_precision_and_scale(result_precision, result_scale)?
@@ -1137,15 +1144,15 @@ fn decimal_op<T: DecimalType>(
}
Op::Rem => {
+ let (p1, s1, p2, s2) = (*p1 as i16, *s1 as i16, *p2 as i16, *s2 as
i16);
// max(s1, s2)
- let result_scale = *s1.max(s2);
+ let result_scale = s1.max(s2);
// min(p1-s1, p2 -s2) + max( s1,s2 )
let result_precision =
- (result_scale.saturating_add((*p1 as i8 - s1).min(*p2 as i8 -
s2)) as u8)
- .min(T::MAX_PRECISION);
+ (result_scale + (p1 - s1).min(p2 - s2)).min(T::MAX_PRECISION
as i16) as u8;
- let l_mul = T::Native::usize_as(10).pow_wrapping((result_scale -
s1) as _);
- let r_mul = T::Native::usize_as(10).pow_wrapping((result_scale -
s2) as _);
+ let l_mul = T::Native::usize_as(10).pow_checked((result_scale -
s1) as _)?;
+ let r_mul = T::Native::usize_as(10).pow_checked((result_scale -
s2) as _)?;
try_op!(
l,
@@ -1154,7 +1161,7 @@ fn decimal_op<T: DecimalType>(
r_s,
l.mul_checked(l_mul)?.mod_checked(r.mul_checked(r_mul)?)
)
- .with_precision_and_scale(result_precision, result_scale)?
+ .with_precision_and_scale(result_precision, result_scale as i8)?
}
};
@@ -1534,6 +1541,115 @@ mod tests {
assert_eq!(err, "Divide by zero error");
}
+ #[test]
+ fn test_decimal256_add_sub_negative_scale_metadata() {
+ let a = Decimal256Array::from(vec![Some(i256::ONE),
Some(i256::MINUS_ONE), None])
+ .with_precision_and_scale(76, -52)
+ .unwrap();
+ let b = Decimal256Array::from(vec![i256::from_i128(2), i256::ONE,
i256::ONE])
+ .with_precision_and_scale(76, -52)
+ .unwrap();
+ let expected =
+ Decimal256Array::from(vec![Some(i256::from_i128(3)),
Some(i256::ZERO), None])
+ .with_precision_and_scale(76, -52)
+ .unwrap();
+ assert_eq!(add(&a, &b).unwrap().as_ref(), &expected);
+
+ let expected =
+ Decimal256Array::from(vec![Some(i256::MINUS_ONE),
Some(i256::from_i128(-2)), None])
+ .with_precision_and_scale(76, -52)
+ .unwrap();
+ assert_eq!(sub(&a, &b).unwrap().as_ref(), &expected);
+ }
+
+ #[test]
+ fn test_decimal256_add_sub_minimum_scale() {
+ let a = Decimal256Array::from(vec![i256::ONE])
+ .with_precision_and_scale(20, i8::MIN)
+ .unwrap();
+ let b = Decimal256Array::from(vec![i256::from_i128(2)])
+ .with_precision_and_scale(20, i8::MIN)
+ .unwrap();
+ let expected = Decimal256Array::from(vec![i256::from_i128(3)])
+ .with_precision_and_scale(21, i8::MIN)
+ .unwrap();
+ assert_eq!(add(&a, &b).unwrap().as_ref(), &expected);
+
+ let expected = Decimal256Array::from(vec![i256::MINUS_ONE])
+ .with_precision_and_scale(21, i8::MIN)
+ .unwrap();
+ assert_eq!(sub(&a, &b).unwrap().as_ref(), &expected);
+ }
+
+ #[test]
+ fn test_decimal256_remainder_negative_scale_metadata() {
+ let a = Decimal256Array::from(vec![Some(i256::ONE),
Some(i256::MINUS_ONE), None])
+ .with_precision_and_scale(76, -52)
+ .unwrap();
+ let b = Decimal256Array::from(vec![i256::from_i128(2), i256::ONE,
i256::ONE])
+ .with_precision_and_scale(76, -52)
+ .unwrap();
+ let expected = Decimal256Array::from(vec![Some(i256::ONE),
Some(i256::ZERO), None])
+ .with_precision_and_scale(76, -52)
+ .unwrap();
+ assert_eq!(rem(&a, &b).unwrap().as_ref(), &expected);
+ }
+
+ #[test]
+ fn test_decimal256_adjacent_minimum_scales() {
+ let a = Decimal256Array::from(vec![i256::ONE])
+ .with_precision_and_scale(20, i8::MIN)
+ .unwrap();
+ let b = Decimal256Array::from(vec![i256::from_i128(2)])
+ .with_precision_and_scale(20, i8::MIN + 1)
+ .unwrap();
+ let expected = Decimal256Array::from(vec![i256::from_i128(12)])
+ .with_precision_and_scale(22, -127)
+ .unwrap();
+ assert_eq!(add(&a, &b).unwrap().as_ref(), &expected);
+
+ let expected = Decimal256Array::from(vec![i256::from_i128(8)])
+ .with_precision_and_scale(22, -127)
+ .unwrap();
+ assert_eq!(sub(&a, &b).unwrap().as_ref(), &expected);
+
+ let expected = Decimal256Array::from(vec![i256::ZERO])
+ .with_precision_and_scale(20, -127)
+ .unwrap();
+ assert_eq!(rem(&a, &b).unwrap().as_ref(), &expected);
+ }
+
+ #[test]
+ fn test_decimal256_extreme_scale_difference_overflow() {
+ let a = Decimal256Array::from(vec![i256::ONE])
+ .with_precision_and_scale(20, i8::MIN)
+ .unwrap();
+ let b = Decimal256Array::from(vec![i256::from_i128(2)])
+ .with_precision_and_scale(76, 76)
+ .unwrap();
+ let expected = "Arithmetic overflow: Overflow happened on: 10 ^ 204";
+ assert_eq!(add(&a, &b).unwrap_err().to_string(), expected);
+ assert_eq!(sub(&a, &b).unwrap_err().to_string(), expected);
+ assert_eq!(rem(&a, &b).unwrap_err().to_string(), expected);
+ }
+
+ #[test]
+ fn test_decimal256_multiply_minimum_scale() {
+ let a = Decimal256Array::from(vec![i256::ONE])
+ .with_precision_and_scale(76, -64)
+ .unwrap();
+ let expected = Decimal256Array::from(vec![i256::ONE])
+ .with_precision_and_scale(76, i8::MIN)
+ .unwrap();
+ assert_eq!(mul(&a, &a).unwrap().as_ref(), &expected);
+
+ let b = a.clone().with_precision_and_scale(76, -65).unwrap();
+ assert_eq!(
+ mul(&a, &b).unwrap_err().to_string(),
+ "Invalid argument error: Output scale of Decimal256(76, -64) *
Decimal256(76, -65) would be less than min scale of -128"
+ );
+ }
+
#[test]
fn test_decimal256_div_wide_intermediate() {
// Dividing two scale-37 values needs l * 10^41, which is 79 digits
and does not
diff --git a/arrow-cast/src/cast/decimal.rs b/arrow-cast/src/cast/decimal.rs
index a5d704bba8..be98b4b4df 100644
--- a/arrow-cast/src/cast/decimal.rs
+++ b/arrow-cast/src/cast/decimal.rs
@@ -172,7 +172,7 @@ where
I::Native: DecimalCast + ArrowNativeTypeOp,
O::Native: DecimalCast + ArrowNativeTypeOp,
{
- let delta_scale = output_scale - input_scale;
+ let delta_scale = output_scale as i16 - input_scale as i16;
// O::MAX_FOR_EACH_PRECISION[k] stores 10^k - 1 (e.g., 9, 99, 999, ...).
// Adding 1 yields exactly 10^k without computing a power at runtime.
@@ -188,7 +188,7 @@ where
// then an increase of scale by 3 will have the following effect on the
representation:
// [xxxxx] -> [xxxxx000], so for the cast to be infallible, the output type
// needs to provide at least 8 digits precision
- let is_infallible_cast = (input_precision as i8) + delta_scale <=
(output_precision as i8);
+ let is_infallible_cast = (input_precision as i16) + delta_scale <=
(output_precision as i16);
let f_infallible = is_infallible_cast
.then_some(move |x|
O::Native::from_decimal(x).unwrap().mul_wrapping(mul));
Some((f_fallible, f_infallible))
@@ -221,7 +221,7 @@ where
I::Native: DecimalCast + ArrowNativeTypeOp,
O::Native: DecimalCast + ArrowNativeTypeOp,
{
- let delta_scale = input_scale - output_scale;
+ let delta_scale = input_scale as i16 - output_scale as i16;
// delta_scale is guaranteed to be > 0, but may also be larger than
I::MAX_PRECISION. If so, the
// scale change divides out more digits than the input has precision and
the result of the cast
@@ -259,7 +259,7 @@ where
// the output type needs to have at least 3 digits of precision.
// e.g. Decimal(5, 3) 99.999 to Decimal(3, 0) will result in 100:
// [99999] -> [99] + 1 = [100], a cast to Decimal(2, 0) would not be
possible
- let is_infallible_cast = (input_precision as i8) - delta_scale <
(output_precision as i8);
+ let is_infallible_cast = (input_precision as i16) - delta_scale <
(output_precision as i16);
let f_infallible = is_infallible_cast.then_some(move |x|
f_fallible(x).unwrap());
Some((f_fallible, f_infallible))
}
@@ -910,6 +910,26 @@ mod tests {
assert_eq!(result, None);
}
+ #[test]
+ fn test_rescale_decimal256_extreme_scales() {
+ assert_eq!(
+ rescale_decimal::<Decimal256Type, Decimal256Type>(i256::ONE, 76,
-76, 76, 0),
+ None
+ );
+ assert_eq!(
+ rescale_decimal::<Decimal256Type, Decimal256Type>(i256::ZERO, 76,
-76, 76, 0),
+ Some(i256::ZERO)
+ );
+ assert_eq!(
+ rescale_decimal::<Decimal256Type, Decimal256Type>(i256::ONE, 76,
i8::MIN, 76, 76),
+ None
+ );
+ assert_eq!(
+ rescale_decimal::<Decimal256Type, Decimal256Type>(i256::ONE, 76,
76, 76, i8::MIN),
+ Some(i256::ZERO)
+ );
+ }
+
#[test]
fn test_rescale_decimal_invalid_input_precision_scale_returns_none() {
let result = rescale_decimal::<Decimal128Type,
Decimal128Type>(123_i128, 39, 39, 38, 38);
diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs
index 1367338405..c02fb900b2 100644
--- a/arrow-cast/src/cast/mod.rs
+++ b/arrow-cast/src/cast/mod.rs
@@ -11255,6 +11255,127 @@ mod tests {
assert!(result.value(0) < 0.0); // Negative result
}
+ #[test]
+ fn test_cast_decimal256_negative_scale_metadata() {
+ let input = Decimal256Array::from(vec![
+ Some(i256::ZERO),
+ Some(i256::ONE),
+ Some(i256::MINUS_ONE),
+ None,
+ ])
+ .with_precision_and_scale(76, -51)
+ .unwrap();
+ let value = i256::from_i128(10).pow_wrapping(51);
+ let expected =
+ Decimal256Array::from(vec![Some(i256::ZERO), Some(value),
Some(-value), None])
+ .with_precision_and_scale(76, 0)
+ .unwrap();
+ let options = CastOptions {
+ safe: false,
+ ..Default::default()
+ };
+ let output_type = DataType::Decimal256(76, 0);
+ let result = cast(&input, &output_type).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+ let result = cast_with_options(&input, &output_type,
&options).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+
+ // The precision plus scale increase crosses i8::MAX at -52.
+ let input = input.with_precision_and_scale(76, -52).unwrap();
+ let value = i256::from_i128(10).pow_wrapping(52);
+ let expected =
+ Decimal256Array::from(vec![Some(i256::ZERO), Some(value),
Some(-value), None])
+ .with_precision_and_scale(76, 0)
+ .unwrap();
+ let result = cast(&input, &output_type).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+ let result = cast_with_options(&input, &output_type,
&options).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+ }
+
+ #[test]
+ fn test_cast_decimal256_negative_scale_precision_overflow() {
+ let input = Decimal256Array::from(vec![Some(i256::ZERO),
Some(i256::ONE), None])
+ .with_precision_and_scale(76, -76)
+ .unwrap();
+ let output_type = DataType::Decimal256(76, 0);
+ let expected = Decimal256Array::from(vec![Some(i256::ZERO), None,
None])
+ .with_precision_and_scale(76, 0)
+ .unwrap();
+ let result = cast(&input, &output_type).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+
+ let options = CastOptions {
+ safe: false,
+ ..Default::default()
+ };
+ let error = cast_with_options(&input, &output_type,
&options).unwrap_err();
+ let value = i256::from_i128(10).pow_wrapping(76);
+ assert_eq!(
+ error.to_string(),
+ format!(
+ "Invalid argument error: {value} is too large to store in a
Decimal256 of precision 76. Max is {}",
+ value - i256::ONE
+ )
+ );
+ }
+
+ #[test]
+ fn test_cast_decimal256_negative_scale_native_overflow() {
+ let input = Decimal256Array::from(vec![Some(i256::from_i128(6)), None])
+ .with_precision_and_scale(76, -76)
+ .unwrap();
+ let output_type = DataType::Decimal256(76, 0);
+ let expected = Decimal256Array::from(vec![None, None])
+ .with_precision_and_scale(76, 0)
+ .unwrap();
+ let result = cast(&input, &output_type).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+
+ let options = CastOptions {
+ safe: false,
+ ..Default::default()
+ };
+ let error = cast_with_options(&input, &output_type,
&options).unwrap_err();
+ assert_eq!(
+ error.to_string(),
+ "Cast error: Cannot cast to Decimal256(76, 0). Overflowing on 6"
+ );
+ }
+
+ #[test]
+ fn test_cast_decimal256_extreme_scale_difference() {
+ let input = Decimal256Array::from(vec![Some(i256::ONE),
Some(i256::MINUS_ONE), None])
+ .with_precision_and_scale(76, i8::MIN)
+ .unwrap();
+ for safe in [true, false] {
+ let options = CastOptions {
+ safe,
+ ..Default::default()
+ };
+ let error =
+ cast_with_options(&input, &DataType::Decimal256(76, 76),
&options).unwrap_err();
+ assert_eq!(
+ error.to_string(),
+ "Cast error: Cannot cast to Decimal256(76, 76). Value
overflows for output scale"
+ );
+ }
+
+ let input = input.with_precision_and_scale(76, 76).unwrap();
+ let expected = Decimal256Array::from(vec![Some(i256::ZERO),
Some(i256::ZERO), None])
+ .with_precision_and_scale(76, i8::MIN)
+ .unwrap();
+ for safe in [true, false] {
+ let options = CastOptions {
+ safe,
+ ..Default::default()
+ };
+ let result =
+ cast_with_options(&input, &DataType::Decimal256(76, i8::MIN),
&options).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+ }
+ }
+
#[test]
fn test_cast_decimal128_to_decimal128_negative_scale() {
let input_type = DataType::Decimal128(20, 0);
