Skip to content

Commit 150bc6f

Browse files
committed
feat(min_max): rename row-scan helper and update match arms
- Renamed the generic row-scan helper back to `min_max_batch_generic` at `min_max.rs:461`. - Removed the `requires_logical_row_scan` indirection. - Updated match arms at `min_max.rs:803` and `min_max.rs:855` to explicitly route `Struct`, `List`, `LargeList`, `FixedSizeList`, and `Dictionary` through `min_max_batch_generic`, including dictionary reuse as requested.
1 parent 377fb5d commit 150bc6f

1 file changed

Lines changed: 11 additions & 20 deletions

File tree

  • datafusion/functions-aggregate-common/src

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

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ macro_rules! min_max {
458458
/// This path is required for dictionary arrays because comparing
459459
/// `dictionary.values()` is not semantically correct: it can include
460460
/// unreferenced values and ignore null key positions.
461-
fn scalar_row_extreme(values: &ArrayRef, ordering: Ordering) -> Result<ScalarValue> {
461+
fn min_max_batch_generic(values: &ArrayRef, ordering: Ordering) -> Result<ScalarValue> {
462462
let mut index = 0;
463463
let mut extreme = loop {
464464
if index == values.len() {
@@ -494,19 +494,6 @@ fn dictionary_scalar_parts(value: &ScalarValue) -> (&ScalarValue, Option<&DataTy
494494
}
495495
}
496496

497-
// Primitive, string, and binary types use specialized Arrow min/max kernels.
498-
// These remaining types fall back to scalar row-by-row logical comparison.
499-
fn requires_logical_row_scan(data_type: &DataType) -> bool {
500-
matches!(
501-
data_type,
502-
DataType::Struct(_)
503-
| DataType::List(_)
504-
| DataType::LargeList(_)
505-
| DataType::FixedSizeList(_, _)
506-
| DataType::Dictionary(_, _)
507-
)
508-
}
509-
510497
/// An accumulator to compute the maximum value
511498
#[derive(Debug, Clone)]
512499
pub struct MaxAccumulator {
@@ -844,9 +831,11 @@ pub fn min_batch(values: &ArrayRef) -> Result<ScalarValue> {
844831
min_binary_view
845832
)
846833
}
847-
data_type if requires_logical_row_scan(data_type) => {
848-
scalar_row_extreme(values, Ordering::Greater)?
849-
}
834+
DataType::Struct(_)
835+
| DataType::List(_)
836+
| DataType::LargeList(_)
837+
| DataType::FixedSizeList(_, _)
838+
| DataType::Dictionary(_, _) => min_max_batch_generic(values, Ordering::Greater)?,
850839
_ => min_max_batch!(values, min),
851840
})
852841
}
@@ -896,9 +885,11 @@ pub fn max_batch(values: &ArrayRef) -> Result<ScalarValue> {
896885
let value = value.map(|e| e.to_vec());
897886
ScalarValue::FixedSizeBinary(*size, value)
898887
}
899-
data_type if requires_logical_row_scan(data_type) => {
900-
scalar_row_extreme(values, Ordering::Less)?
901-
}
888+
DataType::Struct(_)
889+
| DataType::List(_)
890+
| DataType::LargeList(_)
891+
| DataType::FixedSizeList(_, _)
892+
| DataType::Dictionary(_, _) => min_max_batch_generic(values, Ordering::Less)?,
902893
_ => min_max_batch!(values, max),
903894
})
904895
}

0 commit comments

Comments
 (0)