alamb commented on code in PR #8962:
URL: https://github.com/apache/arrow-datafusion/pull/8962#discussion_r1463917955


##########
datafusion/physical-expr/src/functions.rs:
##########
@@ -191,6 +191,30 @@ pub(crate) enum Hint {
     AcceptsSingular,
 }
 
+/// A helper function used to infer the length of arguments of Scalar 
functions and convert
+/// [`ColumnarValue`]s to [`ArrayRef`]s with the inferred length. Note that 
this function
+/// only works for functions that accept either that all arguments are scalars 
or all arguments
+/// are arrays with same length. Otherwise, it will return an error.
+pub fn process_scalar_func_inputs(args: &[ColumnarValue]) -> 
Result<Vec<ArrayRef>> {

Review Comment:
   What would you think about naming this something more descriptive of what it 
does rather than what it is used for? 
   
   Something like this perhaps:
   
   ```suggestion
   pub fn columnar_values_to_array(args: &[ColumnarValue]) -> 
Result<Vec<ArrayRef>> {
   ```
   
   



##########
datafusion/physical-expr/src/functions.rs:
##########
@@ -191,6 +191,30 @@ pub(crate) enum Hint {
     AcceptsSingular,
 }
 
+/// A helper function used to infer the length of arguments of Scalar 
functions and convert
+/// [`ColumnarValue`]s to [`ArrayRef`]s with the inferred length. Note that 
this function
+/// only works for functions that accept either that all arguments are scalars 
or all arguments
+/// are arrays with same length. Otherwise, it will return an error.
+pub fn process_scalar_func_inputs(args: &[ColumnarValue]) -> 
Result<Vec<ArrayRef>> {
+    let len = args
+        .iter()
+        .fold(Option::<usize>::None, |acc, arg| match arg {
+            ColumnarValue::Scalar(_) => acc,
+            ColumnarValue::Array(a) => Some(a.len()),
+        });

Review Comment:
   I think this may also have to handle the case where there are no arguments 
(and needs to return an empty result)



##########
datafusion/physical-expr/src/functions.rs:
##########
@@ -191,6 +191,30 @@ pub(crate) enum Hint {
     AcceptsSingular,
 }
 
+/// A helper function used to infer the length of arguments of Scalar 
functions and convert
+/// [`ColumnarValue`]s to [`ArrayRef`]s with the inferred length. Note that 
this function
+/// only works for functions that accept either that all arguments are scalars 
or all arguments
+/// are arrays with same length. Otherwise, it will return an error.
+pub fn process_scalar_func_inputs(args: &[ColumnarValue]) -> 
Result<Vec<ArrayRef>> {
+    let len = args
+        .iter()
+        .fold(Option::<usize>::None, |acc, arg| match arg {
+            ColumnarValue::Scalar(_) => acc,
+            ColumnarValue::Array(a) => Some(a.len()),
+        });
+
+    let inferred_length = len.ok_or(DataFusionError::Internal(

Review Comment:
   This errors when there are no array arguments, not when there are mixed 
lengths I think 🤔 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to