Re: [PR] Minor: Refactor align_array_dimensions [arrow-datafusion]

["comphead (via GitHub)" <gi...@apache.org>]

comphead commented on code in PR #7963:
URL: https://github.com/apache/arrow-datafusion/pull/7963#discussion_r1375309398


##########
datafusion/physical-expr/src/array_expressions.rs:
##########
@@ -743,35 +743,39 @@ pub fn array_prepend(args: &[ArrayRef]) -> Result<ArrayRef> {
 }
 
 fn align_array_dimensions(args: Vec<ArrayRef>) -> Result<Vec<ArrayRef>> {
-    // Find the maximum number of dimensions
-    let max_ndim: u64 = (*args
-        .iter()
-        .map(|arr| compute_array_ndims(Some(arr.clone())))
-        .collect::<Result<Vec<Option<u64>>>>()?
-        .iter()
-        .max()
-        .unwrap())
-    .unwrap();
+    let mut args_ndim = vec![];
+    for arg in args.iter() {
+        let ndim = compute_array_ndims(Some(arg.to_owned()))?;
+        if let Some(ndim) = ndim {
+            args_ndim.push(ndim);
+        } else {
+            return internal_err!("args should not be empty");
+        }
+    }
+
+    let max_ndim = args_ndim.iter().max();
+    let max_ndim = if let Some(max_ndim) = max_ndim {

Review Comment:
   this check can be earlier, before the loop? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org