Skip to content

Commit 331058d

Browse files
alexyclaude
andcommitted
perf: compare nested ScalarValues by type before converting to ArrayData
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent b40d696 commit 331058d

1 file changed

Lines changed: 63 additions & 7 deletions

File tree

  • datafusion/common/src/scalar

‎datafusion/common/src/scalar/mod.rs‎

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,19 @@ impl Hash for Fl<f16> {
482482
}
483483

484484
// manual implementation of `PartialEq`
485+
/// Equality of the arrays inside nested scalars.
486+
///
487+
/// Arrow's array equality converts both arrays to `ArrayData` before it
488+
/// compares anything, which is costly for wide nested types. Values of
489+
/// different lengths or types are unequal without that, and an array is
490+
/// equal to itself.
491+
fn nested_eq<T: Array + PartialEq>(a: &Arc<T>, b: &Arc<T>) -> bool {
492+
Arc::ptr_eq(a, b)
493+
|| (a.len() == b.len()
494+
&& a.data_type() == b.data_type()
495+
&& a.as_ref() == b.as_ref())
496+
}
497+
485498
impl PartialEq for ScalarValue {
486499
fn eq(&self, other: &Self) -> bool {
487500
use ScalarValue::*;
@@ -552,19 +565,19 @@ impl PartialEq for ScalarValue {
552565
(FixedSizeBinary(_, _), _) => false,
553566
(LargeBinary(v1), LargeBinary(v2)) => v1.eq(v2),
554567
(LargeBinary(_), _) => false,
555-
(FixedSizeList(v1), FixedSizeList(v2)) => v1.eq(v2),
568+
(FixedSizeList(v1), FixedSizeList(v2)) => nested_eq(v1, v2),
556569
(FixedSizeList(_), _) => false,
557-
(List(v1), List(v2)) => v1.eq(v2),
570+
(List(v1), List(v2)) => nested_eq(v1, v2),
558571
(List(_), _) => false,
559-
(LargeList(v1), LargeList(v2)) => v1.eq(v2),
572+
(LargeList(v1), LargeList(v2)) => nested_eq(v1, v2),
560573
(LargeList(_), _) => false,
561-
(ListView(v1), ListView(v2)) => v1.eq(v2),
574+
(ListView(v1), ListView(v2)) => nested_eq(v1, v2),
562575
(ListView(_), _) => false,
563-
(LargeListView(v1), LargeListView(v2)) => v1.eq(v2),
576+
(LargeListView(v1), LargeListView(v2)) => nested_eq(v1, v2),
564577
(LargeListView(_), _) => false,
565-
(Struct(v1), Struct(v2)) => v1.eq(v2),
578+
(Struct(v1), Struct(v2)) => nested_eq(v1, v2),
566579
(Struct(_), _) => false,
567-
(Map(v1), Map(v2)) => v1.eq(v2),
580+
(Map(v1), Map(v2)) => nested_eq(v1, v2),
568581
(Map(_), _) => false,
569582
(Date32(v1), Date32(v2)) => v1.eq(v2),
570583
(Date32(_), _) => false,
@@ -9929,6 +9942,49 @@ mod tests {
99299942
}
99309943
}
99319944

9945+
#[test]
9946+
fn test_nested_scalar_eq() {
9947+
let struct_type = |name: &str| {
9948+
DataType::Struct(Fields::from(vec![
9949+
Field::new(format!("{name}_a"), DataType::Int32, true),
9950+
Field::new(format!("{name}_b"), DataType::Utf8, true),
9951+
]))
9952+
};
9953+
let null_x = ScalarValue::try_from(&struct_type("x")).unwrap();
9954+
let null_x_again = ScalarValue::try_from(&struct_type("x")).unwrap();
9955+
let null_y = ScalarValue::try_from(&struct_type("y")).unwrap();
9956+
assert_eq!(null_x, null_x.clone());
9957+
assert_eq!(null_x, null_x_again);
9958+
assert_ne!(null_x, null_y);
9959+
9960+
let value = |a: i32| {
9961+
ScalarValue::Struct(Arc::new(StructArray::from(vec![
9962+
(
9963+
Arc::new(Field::new("x_a", DataType::Int32, true)),
9964+
Arc::new(Int32Array::from(vec![Some(a)])) as ArrayRef,
9965+
),
9966+
(
9967+
Arc::new(Field::new("x_b", DataType::Utf8, true)),
9968+
Arc::new(StringArray::from(vec![Some("b")])) as ArrayRef,
9969+
),
9970+
])))
9971+
};
9972+
assert_eq!(value(1), value(1));
9973+
assert_ne!(value(1), value(2));
9974+
assert_ne!(value(1), null_x);
9975+
9976+
let list_i32 = ScalarValue::List(ScalarValue::new_list_nullable(
9977+
&[ScalarValue::Int32(Some(1))],
9978+
&DataType::Int32,
9979+
));
9980+
let list_i64 = ScalarValue::List(ScalarValue::new_list_nullable(
9981+
&[ScalarValue::Int64(Some(1))],
9982+
&DataType::Int64,
9983+
));
9984+
assert_eq!(list_i32, list_i32.clone());
9985+
assert_ne!(list_i32, list_i64);
9986+
}
9987+
99329988
#[test]
99339989
fn test_struct_nulls() {
99349990
let fields_b = Fields::from(vec![

0 commit comments

Comments
 (0)