minor: reuse ColumnarValue::into_array in map's expand_if_scalar and avoid uncessary clones

[nathanb9]

Follow-up to #22934: per @alamb's suggestion, reuse the existing ColumnarValue::to_array in map.rs's expand_if_scalar instead of hand-rolling the same logic. to_array (borrowing) is used over into_array to avoid an extra ScalarValue clone; no behavior change.

[alamb]

Thanks @nathanb9

I think we could avoid some clones too like this:

diff --git a/datafusion/functions-nested/src/map.rs b/datafusion/functions-nested/src/map.rs
index 868f6dd290..147f051163 100644
--- a/datafusion/functions-nested/src/map.rs
+++ b/datafusion/functions-nested/src/map.rs
@@ -63,27 +63,19 @@ fn can_evaluate_to_const(args: &[ColumnarValue]) -> bool {
         .all(|arg| matches!(arg, ColumnarValue::Scalar(_)))
 }

-fn expand_if_scalar(arg: &ColumnarValue, rows: usize) -> Result<ColumnarValue> {
-    match arg {
-        ColumnarValue::Scalar(s) => Ok(ColumnarValue::Array(s.to_array_of_size(rows)?)),
-        ColumnarValue::Array(a) => Ok(ColumnarValue::Array(Arc::clone(a))),
-    }
+fn expand_if_scalar(arg: ColumnarValue, rows: usize) -> Result<ColumnarValue> {
+    Ok(ColumnarValue::Array(arg.into_array(rows)?))
 }

-fn make_map_batch(args: &[ColumnarValue], number_rows: usize) -> Result<ColumnarValue> {
-    let [keys_arg, values_arg] = take_function_args("make_map", args)?;
-
-    let can_evaluate_to_const = can_evaluate_to_const(args);
+fn make_map_batch(args: Vec<ColumnarValue>, number_rows: usize) -> Result<ColumnarValue> {
+    let can_evaluate_to_const = can_evaluate_to_const(&args);
+    let [mut keys_arg, mut values_arg] = take_function_args("make_map", args)?;

     // if we can't evaluate to const (inputs are not both scalar) then ensure they
     // are expanded to arrays which following logic expects
-    let (keys_arg, values_arg) = if !can_evaluate_to_const {
-        (
-            expand_if_scalar(keys_arg, number_rows)?,
-            expand_if_scalar(values_arg, number_rows)?,
-        )
-    } else {
-        (keys_arg.clone(), values_arg.clone())
+    if !can_evaluate_to_const {
+        keys_arg = expand_if_scalar(keys_arg, number_rows)?;
+        values_arg = expand_if_scalar(values_arg, number_rows)?;
     };

     let keys = get_first_array_ref(&keys_arg)?;
@@ -417,7 +409,7 @@ impl ScalarUDFImpl for MapFunc {
     }

     fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result<ColumnarValue> {
-        make_map_batch(&args.args, args.number_rows)
+        make_map_batch(args.args, args.number_rows)
     }

[nathanb9]

Thanks. I added suggested changes

I'd swapped the one clone() for to_array to avoid that call, but I see what you mean. We can just have expand_if_scalar take ownership and use into_array with no clones.

[alamb]

Thank you @nathanb9