[GitHub] [arrow-datafusion] avantgardnerio commented on a diff in pull request #5380: UDF zero params #5378

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

avantgardnerio commented on code in PR #5380:
URL: https://github.com/apache/arrow-datafusion/pull/5380#discussion_r1117352997


##########
datafusion/physical-expr/src/planner.rs:
##########
@@ -393,7 +393,10 @@ pub fn create_physical_expr(
                     execution_props,
                 )?);
             }
-
+            // udfs with zero params expect null array as input
+            if args.is_empty() {
+                physical_args.push(Arc::new(Literal::new(ScalarValue::Null)));

Review Comment:
   Though a convenient implementation, I'm wary that generating a fake argument by which to transfer row count will cause problems later.
   
   I'd prefer to see a solution where the row count is passed out-of-band, for example in a change to https://github.com/apache/arrow-datafusion/blob/cef119da9ee8672b1b1e50ac01387dcb1640d96e/datafusion/expr/src/function.rs#L39 that would add an extra argument (i.e. `len: usize`) for this purpose.
   
   If that were present, we could populate it up in the call stack where we know the RecordBatch size, probably here: https://github.com/apache/arrow-datafusion/blob/cef119da9ee8672b1b1e50ac01387dcb1640d96e/datafusion/physical-expr/src/scalar_function.rs#L147



-- 
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