Simplify min/max flow in dictionary handling

Refactor dictionary min/max flow by removing the wrap macro arm,
making re-wrapping explicit through a private helper. This
separates the "choose inner winner" from the "wrap as
dictionary" step for easier auditing.

In `datafusion/functions-aggregate/src/min_max.rs`, update
`string_dictionary_batch` to accept slices instead of owned
Vecs, and introduce a small `evaluate_dictionary_accumulator`
helper to streamline min/max assertions with a shared
accumulator execution path, reducing repeated setup.

Author: kosiew

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

@@ -142,13 +142,7 @@ macro_rules! min_max_generic {
 }
 
 macro_rules! min_max_dictionary {
-    ($VALUE:expr, $DELTA:expr, wrap $KEY_TYPE:expr, $OP:ident) => {{
-        let winner = min_max_generic!($VALUE, $DELTA, $OP);
-        ScalarValue::Dictionary($KEY_TYPE.clone(), Box::new(winner))
-    }};
-    ($VALUE:expr, $DELTA:expr, $OP:ident) => {{
-        min_max_generic!($VALUE, $DELTA, $OP)
-    }};
+    ($VALUE:expr, $DELTA:expr, $OP:ident) => {{ min_max_generic!($VALUE, $DELTA, $OP) }};
 }
 
 // min/max of two scalar values of the same type
@@ -427,11 +421,13 @@ macro_rules! min_max {
                 ScalarValue::Dictionary(key_type, lhs_inner),
                 ScalarValue::Dictionary(_, rhs_inner),
             ) => {
-                min_max_dictionary!(
+                wrap_dictionary_scalar(
+                    key_type.as_ref(),
+                    min_max_dictionary!(
                     lhs_inner.as_ref(),
                     rhs_inner.as_ref(),
-                    wrap key_type,
                     $OP
+                    ),
                 )
             }
 
@@ -467,7 +463,11 @@ fn dictionary_batch_extreme(
         unreachable!("dictionary_batch_extreme requires dictionary arrays")
     };
     let inner = extreme_fn(values.as_any_dictionary().values())?;
-    Ok(ScalarValue::Dictionary(key_type.clone(), Box::new(inner)))
+    Ok(wrap_dictionary_scalar(key_type.as_ref(), inner))
+}
+
+fn wrap_dictionary_scalar(key_type: &DataType, value: ScalarValue) -> ScalarValue {
+    ScalarValue::Dictionary(Box::new(key_type.clone()), Box::new(value))
 }
 
 /// An accumulator to compute the maximum value

datafusion/functions-aggregate/src/min_max.rs

@@ -1271,13 +1271,21 @@ mod tests {
         dict_scalar(key_type, ScalarValue::Utf8(Some(value.to_string())))
     }
 
-    fn string_dictionary_batch(
-        values: Vec<&str>,
-        keys: Vec<Option<i32>>,
-    ) -> ArrayRef {
-        let values = Arc::new(StringArray::from(values)) as ArrayRef;
-        Arc::new(DictionaryArray::try_new(Int32Array::from(keys), values).unwrap())
-            as ArrayRef
+    fn string_dictionary_batch(values: &[&str], keys: &[Option<i32>]) -> ArrayRef {
+        let values = Arc::new(StringArray::from(values.to_vec())) as ArrayRef;
+        Arc::new(
+            DictionaryArray::try_new(Int32Array::from(keys.to_vec()), values).unwrap(),
+        ) as ArrayRef
+    }
+
+    fn evaluate_dictionary_accumulator(
+        mut acc: impl Accumulator,
+        batches: &[ArrayRef],
+    ) -> Result<ScalarValue> {
+        for batch in batches {
+            acc.update_batch(&[Arc::clone(batch)])?;
+        }
+        acc.evaluate()
     }
 
     fn assert_dictionary_min_max(
@@ -1291,29 +1299,26 @@ mod tests {
             other => panic!("expected dictionary type, got {other:?}"),
         };
 
-        let mut min_acc = MinAccumulator::try_new(dict_type)?;
-        for batch in batches {
-            min_acc.update_batch(&[Arc::clone(batch)])?;
-        }
-        assert_eq!(
-            min_acc.evaluate()?,
-            utf8_dict_scalar(key_type.clone(), expected_min)
-        );
+        let min_result = evaluate_dictionary_accumulator(
+            MinAccumulator::try_new(dict_type)?,
+            batches,
+        )?;
+        assert_eq!(min_result, utf8_dict_scalar(key_type.clone(), expected_min));
 
-        let mut max_acc = MaxAccumulator::try_new(dict_type)?;
-        for batch in batches {
-            max_acc.update_batch(&[Arc::clone(batch)])?;
-        }
-        assert_eq!(max_acc.evaluate()?, utf8_dict_scalar(key_type, expected_max));
+        let max_result = evaluate_dictionary_accumulator(
+            MaxAccumulator::try_new(dict_type)?,
+            batches,
+        )?;
+        assert_eq!(max_result, utf8_dict_scalar(key_type, expected_max));
 
         Ok(())
     }
 
     #[test]
     fn test_min_max_dictionary_without_coercion() -> Result<()> {
         let dict_array_ref = string_dictionary_batch(
-            vec!["b", "c", "a", "d"],
-            vec![Some(0), Some(1), Some(2), Some(3)],
+            &["b", "c", "a", "d"],
+            &[Some(0), Some(1), Some(2), Some(3)],
         );
         let dict_type = dict_array_ref.data_type().clone();
 
@@ -1323,8 +1328,8 @@ mod tests {
     #[test]
     fn test_min_max_dictionary_with_nulls() -> Result<()> {
         let dict_array_ref = string_dictionary_batch(
-            vec!["b", "c", "a"],
-            vec![None, Some(0), None, Some(1), Some(2)],
+            &["b", "c", "a"],
+            &[None, Some(0), None, Some(1), Some(2)],
         );
         let dict_type = dict_array_ref.data_type().clone();
 
@@ -1335,10 +1340,8 @@ mod tests {
     fn test_min_max_dictionary_multi_batch() -> Result<()> {
         let dict_type =
             DataType::Dictionary(Box::new(DataType::Int32), Box::new(DataType::Utf8));
-        let batch1 =
-            string_dictionary_batch(vec!["b", "c"], vec![Some(0), Some(1)]);
-        let batch2 =
-            string_dictionary_batch(vec!["a", "d"], vec![Some(0), Some(1)]);
+        let batch1 = string_dictionary_batch(&["b", "c"], &[Some(0), Some(1)]);
+        let batch2 = string_dictionary_batch(&["a", "d"], &[Some(0), Some(1)]);
 
         assert_dictionary_min_max(&dict_type, &[batch1, batch2], "a", "d")
     }