Address review comments II

Author: erenavsarogullari

datafusion/functions-aggregate/src/min_max.rs

@@ -574,7 +574,7 @@ fn min_batch(values: &ArrayRef) -> Result<ScalarValue> {
 }
 
 /// dynamically-typed max(array) -> ScalarValue
-fn max_batch(values: &ArrayRef) -> Result<ScalarValue> {
+pub fn max_batch(values: &ArrayRef) -> Result<ScalarValue> {
     Ok(match values.data_type() {
         DataType::Utf8 => {
             typed_min_max_batch_string!(values, StringArray, Utf8, max_string)

datafusion/functions-nested/src/max.rs

@@ -16,18 +16,18 @@
 // under the License.
 
 //! [`ScalarUDFImpl`] definitions for array_max function.
-use crate::sort::array_sort_inner;
 use crate::utils::make_scalar_function;
-use arrow_array::{Array, ArrayRef, StringArray};
+use arrow_array::{Array, ArrayRef};
 use arrow_schema::DataType;
 use arrow_schema::DataType::{FixedSizeList, LargeList, List};
 use datafusion_common::cast::as_list_array;
 use datafusion_common::exec_err;
 use datafusion_doc::Documentation;
 use datafusion_expr::{ColumnarValue, ScalarUDFImpl, Signature, Volatility};
+use datafusion_functions::utils::take_function_args;
+use datafusion_functions_aggregate::min_max;
 use datafusion_macros::user_doc;
 use std::any::Any;
-use std::sync::Arc;
 
 make_udf_expr_and_func!(
     ArrayMax,
@@ -124,29 +124,14 @@ impl ScalarUDFImpl for ArrayMax {
 /// For example:
 /// > array_max(\[1, 3, 2]) -> 3
 pub fn array_max_inner(args: &[ArrayRef]) -> datafusion_common::Result<ArrayRef> {
-    if args.len() != 1 {
-        return exec_err!("array_max needs one argument");
-    }
+    let [arg1] = take_function_args("array_max", args)?;
 
-    match &args[0].data_type() {
+    match arg1.data_type() {
         List(_) | LargeList(_) | FixedSizeList(_, _) => {
-            let new_args = vec![
-                Arc::<dyn Array>::clone(&args[0]),
-                Arc::new(StringArray::from_iter(vec![Some("DESC")])),
-                Arc::new(StringArray::from_iter(vec![Some("NULLS LAST")])),
-            ];
-            array_max_internal(&new_args)
+            let input_array = as_list_array(&arg1)?.value(0);
+            let max_result = min_max::max_batch(&input_array);
+            max_result?.to_array()
         }
-        _ => exec_err!("array_max does not support type: {:?}", args[0].data_type()),
-    }
-}
-
-fn array_max_internal(args: &[ArrayRef]) -> datafusion_common::Result<ArrayRef> {
-    let sorted_array = array_sort_inner(args)?;
-    let result_array = as_list_array(&sorted_array)?.value(0);
-    if result_array.is_empty() {
-        return exec_err!("array_max needs one argument as non-empty array");
+        _ => exec_err!("array_max does not support type: {:?}", arg1.data_type()),
     }
-    let max_result = result_array.slice(0, 1);
-    Ok(max_result)
 }

datafusion/sqllogictest/test_files/array.slt

@@ -1491,8 +1491,20 @@ select array_max(make_array(NULL, TIMESTAMP '1996-10-01', TIMESTAMP '1995-06-01'
 ----
 1996-10-01T00:00:00
 
-query error Execution error: array_max needs one argument as non-empty array
+query R
+select array_max(make_array(5.1, -3.2, 6.3, 4.9));
+----
+6.3
+
+query I
 select array_max(make_array());
+----
+NULL
+
+# Testing with empty arguments should result in an error
+query error DataFusion error: Error during planning: 'array_max' does not support zero arguments
+select array_max();
+
 
 ## array_pop_back (aliases: `list_pop_back`)