Skip to content

Commit b92aeef

Browse files
committed
fix(min_max): rename helper to scalar_row_extreme and update documentation
- Renamed the helper function from `scalar_batch_extreme` to `scalar_row_extreme` - Added a doc comment explaining that this helper scans logical rows and the necessity of this for dictionary arrays (noting that comparing dictionary.values() is not semantically correct). - Updated both call sites to use the new helper name in min/max batch logic at min_max.rs
1 parent dad6e02 commit b92aeef

1 file changed

Lines changed: 8 additions & 3 deletions

File tree

  • datafusion/functions-aggregate-common/src

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

Lines changed: 8 additions & 3 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() {
@@ -824,7 +829,7 @@ pub fn min_batch(values: &ArrayRef) -> Result<ScalarValue> {
824829
)
825830
}
826831
data_type if is_row_wise_batch_type(data_type) => {
827-
scalar_batch_extreme(values, Ordering::Greater)?
832+
scalar_row_extreme(values, Ordering::Greater)?
828833
}
829834
_ => min_max_batch!(values, min),
830835
})
@@ -876,7 +881,7 @@ pub fn max_batch(values: &ArrayRef) -> Result<ScalarValue> {
876881
ScalarValue::FixedSizeBinary(*size, value)
877882
}
878883
data_type if is_row_wise_batch_type(data_type) => {
879-
scalar_batch_extreme(values, Ordering::Less)?
884+
scalar_row_extreme(values, Ordering::Less)?
880885
}
881886
_ => min_max_batch!(values, max),
882887
})

0 commit comments

Comments
 (0)