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 edd277530b fix: Reject decimal arrays with different scales in
`make_comparator` (#10864)
edd277530b is described below
commit edd277530b85070e30402cb390fd89a6751bb9f3
Author: Neil Conway <[email protected]>
AuthorDate: Thu Aug 27 22:25:00 2026 -0400
fix: Reject decimal arrays with different scales in `make_comparator`
(#10864)
# Which issue does this PR close?
- Closes #10863.
# Rationale for this change
`make_comparator` accepted decimal arrays with different scales and
compared their raw, unscaled values. This yields incorrect results.
Instead, reject such inputs, as we do for other instances of
incomparable types. There isn't a clean way to do this inside the
existing per-type `downcast_primitive!` structure (since that discards
the scale), so add a check on the scale before we match on the array
type.
Note that precision is not required to match: precision only bounds what
can be stored in a given decimal value, it does not influence the actual
bitwise representation of a decimal value.
Most downstream systems will avoid calling `make_comparator` on decimals
with mismatched scales, but if this situation does occur, an error is
much better than silently returning incorrect results.
# What changes are included in this PR?
See above.
# Are these changes tested?
Yes; new tests added.
# Are there any user-facing changes?
No. (Previously allowed inputs to `make_comparator` will now be
rejected, but such inputs did not yield a sensible result in the past.)
# AI usage
Claude Code Fable 5 found the bug and wrote the fix; I reviewed, edited,
and understand the resulting code.
---
arrow-cmp/src/lib.rs | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/arrow-cmp/src/lib.rs b/arrow-cmp/src/lib.rs
index d3ef0058e3..6772e64222 100644
--- a/arrow-cmp/src/lib.rs
+++ b/arrow-cmp/src/lib.rs
@@ -459,6 +459,8 @@ fn compare_union(
/// If `nulls_first` is true, null values are considered less than any non-null
/// value; otherwise they are considered greater. This is primarily shared by
/// crates that need repeated slot comparisons without constructing sliced
arrays.
+///
+/// Returns an error for arrays whose types are not comparable.
pub fn make_comparator(
left: &dyn Array,
right: &dyn Array,
@@ -466,6 +468,24 @@ pub fn make_comparator(
) -> Result<DynComparator, ArrowError> {
use arrow_schema::DataType::*;
+ // Decimal arrays of the same width are compared as raw unscaled values
+ // below; that is only correct when the scales match.
+ match (left.data_type(), right.data_type()) {
+ (Decimal32(_, s1), Decimal32(_, s2))
+ | (Decimal64(_, s1), Decimal64(_, s2))
+ | (Decimal128(_, s1), Decimal128(_, s2))
+ | (Decimal256(_, s1), Decimal256(_, s2))
+ if s1 != s2 =>
+ {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Can't compare decimal arrays with different scales: {:?},
{:?}",
+ left.data_type(),
+ right.data_type()
+ )));
+ }
+ _ => {}
+ }
+
macro_rules! primitive_helper {
($t:ty, $left:expr, $right:expr, $nulls_first:expr) => {
Ok(compare_primitive::<$t>($left, $right, $nulls_first))
@@ -918,6 +938,63 @@ mod tests {
assert_eq!(Ordering::Greater, cmp(3, 2));
}
+ #[test]
+ fn test_decimal_different_scale_errors() {
+ // https://github.com/apache/arrow-rs/issues/10863
+ let a = Decimal128Array::from(vec![100])
+ .with_precision_and_scale(10, 2)
+ .unwrap();
+ let b = Decimal128Array::from(vec![500])
+ .with_precision_and_scale(10, 3)
+ .unwrap();
+ let err = make_comparator(&a, &b, SortOptions::default())
+ .err()
+ .unwrap();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: Can't compare decimal arrays with
different scales: \
+ Decimal128(10, 2), Decimal128(10, 3)"
+ );
+
+ // the same check applies behind nested types
+ let keys = Int8Array::from_iter_values([0]);
+ let dict_a = DictionaryArray::new(keys.clone(), Arc::new(a.clone()));
+ let dict_b = DictionaryArray::new(keys, Arc::new(b));
+ assert!(make_comparator(&dict_a, &dict_b,
SortOptions::default()).is_err());
+
+ // precision does not affect the ordering, so it need not match
+ let c = Decimal128Array::from(vec![100])
+ .with_precision_and_scale(12, 2)
+ .unwrap();
+ let cmp = make_comparator(&a, &c, SortOptions::default()).unwrap();
+ assert_eq!(Ordering::Equal, cmp(0, 0));
+
+ // every decimal width is checked
+ let a = Decimal32Array::from(vec![1])
+ .with_precision_and_scale(5, 1)
+ .unwrap();
+ let b = Decimal32Array::from(vec![1])
+ .with_precision_and_scale(5, 2)
+ .unwrap();
+ assert!(make_comparator(&a, &b, SortOptions::default()).is_err());
+
+ let a = Decimal64Array::from(vec![1])
+ .with_precision_and_scale(10, 1)
+ .unwrap();
+ let b = Decimal64Array::from(vec![1])
+ .with_precision_and_scale(10, 2)
+ .unwrap();
+ assert!(make_comparator(&a, &b, SortOptions::default()).is_err());
+
+ let a = Decimal256Array::from(vec![i256::from_i128(1)])
+ .with_precision_and_scale(40, 1)
+ .unwrap();
+ let b = Decimal256Array::from(vec![i256::from_i128(1)])
+ .with_precision_and_scale(40, 2)
+ .unwrap();
+ assert!(make_comparator(&a, &b, SortOptions::default()).is_err());
+ }
+
fn test_bytes_impl<T: ByteArrayType>() {
let offsets = OffsetBuffer::from_lengths([3, 3, 1]);
let a = GenericByteArray::<T>::new(offsets, b"abcdefa".into(), None);
