jayzhan211 commented on code in PR #26006:
URL: https://github.com/apache/datafusion/pull/26006#discussion_r4370081685
##########
datafusion/physical-plan/src/joins/utils.rs:
##########
@@ -2664,88 +2664,9 @@ pub fn compare_join_arrays(
sort_options: &[SortOptions],
null_equality: NullEquality,
) -> Result<Ordering> {
- let mut res = Ordering::Equal;
- for ((left_array, right_array), sort_options) in
- left_arrays.iter().zip(right_arrays).zip(sort_options)
- {
- macro_rules! compare_value {
- ($T:ty) => {{
- let left_array =
left_array.as_any().downcast_ref::<$T>().unwrap();
- let right_array =
right_array.as_any().downcast_ref::<$T>().unwrap();
- match (left_array.is_null(left), right_array.is_null(right)) {
- (false, false) => {
- let left_value = &left_array.value(left);
- let right_value = &right_array.value(right);
- res = left_value.partial_cmp(right_value).unwrap();
- if sort_options.descending {
- res = res.reverse();
- }
- }
- (true, false) => {
- res = if sort_options.nulls_first {
- Ordering::Less
- } else {
- Ordering::Greater
- };
- }
- (false, true) => {
- res = if sort_options.nulls_first {
- Ordering::Greater
- } else {
- Ordering::Less
- };
- }
- _ => {
- res = match null_equality {
- NullEquality::NullEqualsNothing => Ordering::Less,
- NullEquality::NullEqualsNull => Ordering::Equal,
- };
- }
- }
- }};
- }
-
- match left_array.data_type() {
- DataType::Null => {}
- DataType::Boolean => compare_value!(BooleanArray),
- DataType::Int8 => compare_value!(Int8Array),
- DataType::Int16 => compare_value!(Int16Array),
- DataType::Int32 => compare_value!(Int32Array),
- DataType::Int64 => compare_value!(Int64Array),
- DataType::UInt8 => compare_value!(UInt8Array),
- DataType::UInt16 => compare_value!(UInt16Array),
- DataType::UInt32 => compare_value!(UInt32Array),
- DataType::UInt64 => compare_value!(UInt64Array),
- DataType::Float32 => compare_value!(Float32Array),
- DataType::Float64 => compare_value!(Float64Array),
- DataType::Binary => compare_value!(BinaryArray),
- DataType::BinaryView => compare_value!(BinaryViewArray),
- DataType::FixedSizeBinary(_) =>
compare_value!(FixedSizeBinaryArray),
- DataType::LargeBinary => compare_value!(LargeBinaryArray),
- DataType::Utf8 => compare_value!(StringArray),
- DataType::Utf8View => compare_value!(StringViewArray),
- DataType::LargeUtf8 => compare_value!(LargeStringArray),
- DataType::Decimal128(..) => compare_value!(Decimal128Array),
- DataType::Timestamp(time_unit, None) => match time_unit {
- TimeUnit::Second => compare_value!(TimestampSecondArray),
- TimeUnit::Millisecond =>
compare_value!(TimestampMillisecondArray),
- TimeUnit::Microsecond =>
compare_value!(TimestampMicrosecondArray),
- TimeUnit::Nanosecond =>
compare_value!(TimestampNanosecondArray),
- },
- DataType::Date32 => compare_value!(Date32Array),
- DataType::Date64 => compare_value!(Date64Array),
- dt => {
- return not_impl_err!(
- "Unsupported data type in sort merge join comparator: {}",
- dt
- );
- }
- }
- if !res.is_eq() {
- break;
- }
- }
- Ok(res)
+ let cmp =
Review Comment:
nit: nothing in this crate calls `compare_join_arrays` now, but it's still
public. Building the comparator over the full arrays makes every call O(len):
`normalize_float_zero` scans all the float values and `logical_nulls` scans
dictionary keys. Slicing both sides to the one row first keeps it O(1) per
call, like before:
```diff
- let cmp =
- JoinKeyComparator::new(left_arrays, right_arrays, sort_options,
null_equality)?;
- Ok(cmp.compare(left, right))
+ let left_row: Vec<ArrayRef> = left_arrays.iter().map(|a| a.slice(left,
1)).collect();
+ let right_row: Vec<ArrayRef> =
+ right_arrays.iter().map(|a| a.slice(right, 1)).collect();
+ let cmp = JoinKeyComparator::new(&left_row, &right_row, sort_options,
null_equality)?;
+ Ok(cmp.compare(0, 0))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
