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 bb0d497b75 fix: casting Decimal256 to signed integers returned wrong
results (#10857)
bb0d497b75 is described below
commit bb0d497b75dbbd2988a1fe527c6e97b5b49b0209
Author: Neil Conway <[email protected]>
AuthorDate: Wed Aug 26 20:58:12 2026 -0400
fix: casting Decimal256 to signed integers returned wrong results (#10857)
# Which issue does this PR close?
- Closes #10855.
# Rationale for this change
`ToPrimitive::to_i64` for `i256` re-tested the already-validated top 128
bits instead of bits 64..127, so any value whose two 64-bit halves agree
in sign was truncated to its low 64 bits. This resulted in incorrect
results when casting Decimal256 to a signed integer type.
Fix it by rewriting `i256::to_i64` in terms of `i256::to_i128`, which
was already correct. Also rewrite `i256::to_u64` in a similar manner,
for consistency. Benchmarking confirms that this approach yields
~equivalent performance to the previous implementation, but removes
redundancy and fixes the bug.
# What changes are included in this PR?
* Implement `to_i64` and `to_u64` in terms of `i256::to_i128`
* Add tests, both for `ToPrimitive` and casting-level behavior
# Are these changes tested?
Yes, new tests added.
# Are there any user-facing changes?
No, aside from fixing incorrect behavior.
# AI usage
Bug found and fix developed with Claude Code Fable; I reviewed and
understand the resulting code.
---
arrow-buffer/src/bigint/mod.rs | 58 ++++++++++++++++++++----------------------
arrow-cast/src/cast/mod.rs | 28 ++++++++++++++++++++
2 files changed, 56 insertions(+), 30 deletions(-)
diff --git a/arrow-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs
index f774a7a5d7..6811e2e1ed 100644
--- a/arrow-buffer/src/bigint/mod.rs
+++ b/arrow-buffer/src/bigint/mod.rs
@@ -1081,25 +1081,7 @@ define_as_primitive!(u64);
impl ToPrimitive for i256 {
fn to_i64(&self) -> Option<i64> {
- let as_i128 = self.low as i128;
-
- let high_negative = self.high < 0;
- let low_negative = as_i128 < 0;
- let high_valid = self.high == -1 || self.high == 0;
-
- if high_negative == low_negative && high_valid {
- let (low_bytes, high_bytes) =
split_array(u128::to_le_bytes(self.low));
- let high = i64::from_le_bytes(high_bytes);
- let low = i64::from_le_bytes(low_bytes);
-
- let high_negative = high < 0;
- let low_negative = low < 0;
- let high_valid = self.high == -1 || self.high == 0;
-
- (high_negative == low_negative && high_valid).then_some(low)
- } else {
- None
- }
+ i64::try_from(i256::to_i128(*self)?).ok()
}
fn to_f64(&self) -> Option<f64> {
@@ -1112,17 +1094,7 @@ impl ToPrimitive for i256 {
}
fn to_u64(&self) -> Option<u64> {
- let as_i128 = self.low as i128;
-
- let high_negative = self.high < 0;
- let low_negative = as_i128 < 0;
- let high_valid = self.high == -1 || self.high == 0;
-
- if high_negative == low_negative && high_valid {
- self.low.to_u64()
- } else {
- None
- }
+ u64::try_from(i256::to_i128(*self)?).ok()
}
}
@@ -1694,6 +1666,32 @@ mod tests {
let a = i256::from_i128(i64::MIN as i128 - 1);
assert!(a.to_i64().is_none());
assert!(a.to_u64().is_none());
+
+ // values whose two 64-bit halves agree in sign but exceed i64/u64
+ // https://github.com/apache/arrow-rs/issues/10855
+ let a = i256::from_i128((1i128 << 64) + 5);
+ assert!(a.to_i64().is_none());
+ assert!(a.to_u64().is_none());
+ assert!(a.to_i32().is_none());
+ assert!(a.to_i8().is_none());
+
+ let a = i256::from_i128(-((1i128 << 64) + 5));
+ assert!(a.to_i64().is_none());
+ assert!(a.to_u64().is_none());
+ assert!(a.to_i32().is_none());
+ assert!(a.to_i8().is_none());
+
+ let a = i256::from_i128(u64::MAX as i128);
+ assert!(a.to_i64().is_none());
+ assert_eq!(a.to_u64().unwrap(), u64::MAX);
+
+ let a = i256::from_parts(5, 1);
+ assert!(a.to_i64().is_none());
+ assert!(a.to_u64().is_none());
+
+ let a = i256::from_parts(u64::MAX as u128 + 5, 0);
+ assert!(a.to_i64().is_none());
+ assert!(a.to_u64().is_none());
}
#[test]
diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs
index 1d95d2dd36..52733422ea 100644
--- a/arrow-cast/src/cast/mod.rs
+++ b/arrow-cast/src/cast/mod.rs
@@ -4097,6 +4097,34 @@ mod tests {
assert!(casted_array.is_ok());
assert!(casted_array.unwrap().is_null(0));
+ // overflow test: values whose low 64 bits fit the target type
+ // https://github.com/apache/arrow-rs/issues/10855
+ let value_array: Vec<Option<i256>> = vec![Some(i256::from_i128((1i128
<< 64) + 5))];
+ let array = create_decimal256_array(value_array, 76, 0).unwrap();
+ let casted_array = cast_with_options(
+ &array,
+ &DataType::Int64,
+ &CastOptions {
+ safe: false,
+ format_options: FormatOptions::default(),
+ },
+ );
+ assert_eq!(
+ "Cast error: value of 18446744073709551621 is out of range
Int64".to_string(),
+ casted_array.unwrap_err().to_string()
+ );
+
+ let casted_array = cast_with_options(
+ &array,
+ &DataType::Int64,
+ &CastOptions {
+ safe: true,
+ format_options: FormatOptions::default(),
+ },
+ );
+ assert!(casted_array.is_ok());
+ assert!(casted_array.unwrap().is_null(0));
+
// loss the precision: convert decimal to f32、f64
// f32
// 112345678_f32 and 112345679_f32 are same, so the 112345679_f32 will
lose precision.
