jayzhan211 commented on code in PR #10193: URL: https://github.com/apache/datafusion/pull/10193#discussion_r1580245132
########## datafusion/physical-expr/src/scalar_function.rs: ########## @@ -142,25 +137,21 @@ impl PhysicalExpr for ScalarFunctionExpr { } fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> { - // evaluate the arguments, if there are no arguments we'll instead pass in a null array - // indicating the batch size (as a convention) - let inputs = match self.args.is_empty() { - // If the function supports zero argument, we pass in a null array indicating the batch size. - // This is for user-defined functions. - // MakeArray support zero argument but has the different behavior from the array with one null. - true if self.supports_zero_argument && self.name != "make_array" => { - vec![ColumnarValue::create_null_array(batch.num_rows())] - } - _ => self - .args - .iter() - .map(|e| e.evaluate(batch)) - .collect::<Result<Vec<_>>>()?, - }; + let inputs = self + .args + .iter() + .map(|e| e.evaluate(batch)) + .collect::<Result<Vec<_>>>()?; // evaluate the function match self.fun { - ScalarFunctionDefinition::UDF(ref fun) => fun.invoke(&inputs), + ScalarFunctionDefinition::UDF(ref fun) => { + if fun.support_randomness() { + fun.invoke_no_args(batch.num_rows()) Review Comment: Functions that include randomness don't imply 0 args at all. I think it is better to call `invoke_no_args` if it is no args 💯 -- 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-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org