Skip to content

Commit 963523d

Browse files
committed
Revert to a80fc77: feat(min_max): rename predicate to requires_logical_row_scan
1 parent d5024c8 commit 963523d

1 file changed

Lines changed: 13 additions & 6 deletions

File tree

  • datafusion/functions-aggregate-common/src

datafusion/functions-aggregate-common/src/min_max.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,12 @@ macro_rules! min_max {
439439
}};
440440
}
441441

442-
fn scalar_batch_extreme(values: &ArrayRef, ordering: Ordering) -> Result<ScalarValue> {
442+
/// Finds the min/max by scanning logical rows via `ScalarValue::try_from_array`.
443+
///
444+
/// This path is required for dictionary arrays because comparing
445+
/// `dictionary.values()` is not semantically correct: it can include
446+
/// unreferenced values and ignore null key positions.
447+
fn scalar_row_extreme(values: &ArrayRef, ordering: Ordering) -> Result<ScalarValue> {
443448
let mut index = 0;
444449
let mut extreme = loop {
445450
if index == values.len() {
@@ -475,7 +480,9 @@ fn dictionary_scalar_parts(value: &ScalarValue) -> (&ScalarValue, Option<&DataTy
475480
}
476481
}
477482

478-
fn is_row_wise_batch_type(data_type: &DataType) -> bool {
483+
// Primitive, string, and binary types use specialized Arrow min/max kernels.
484+
// These remaining types fall back to scalar row-by-row logical comparison.
485+
fn requires_logical_row_scan(data_type: &DataType) -> bool {
479486
matches!(
480487
data_type,
481488
DataType::Struct(_)
@@ -823,8 +830,8 @@ pub fn min_batch(values: &ArrayRef) -> Result<ScalarValue> {
823830
min_binary_view
824831
)
825832
}
826-
data_type if is_row_wise_batch_type(data_type) => {
827-
scalar_batch_extreme(values, Ordering::Greater)?
833+
data_type if requires_logical_row_scan(data_type) => {
834+
scalar_row_extreme(values, Ordering::Greater)?
828835
}
829836
_ => min_max_batch!(values, min),
830837
})
@@ -875,8 +882,8 @@ pub fn max_batch(values: &ArrayRef) -> Result<ScalarValue> {
875882
let value = value.map(|e| e.to_vec());
876883
ScalarValue::FixedSizeBinary(*size, value)
877884
}
878-
data_type if is_row_wise_batch_type(data_type) => {
879-
scalar_batch_extreme(values, Ordering::Less)?
885+
data_type if requires_logical_row_scan(data_type) => {
886+
scalar_row_extreme(values, Ordering::Less)?
880887
}
881888
_ => min_max_batch!(values, max),
882889
})

0 commit comments

Comments
 (0)