ZhangHuiGui commented on code in PR #40396:
URL: https://github.com/apache/arrow/pull/40396#discussion_r1518484854


##########
cpp/src/arrow/compute/expression.cc:
##########
@@ -845,7 +845,8 @@ Result<Expression> FoldConstants(Expression expr) {
       std::move(expr), [](Expression expr) { return expr; },
       [](Expression expr, ...) -> Result<Expression> {
         auto call = CallNotNull(expr);
-        if (std::all_of(call->arguments.begin(), call->arguments.end(),
+        if (!call->arguments.empty() &&

Review Comment:
   Thanks for your review. The `is_impure` is a better way to decide whether a 
call itself need simplified, and we need to consider the functions with no 
arguments during registered(random, hash_count_all, count_all, udf-functions) . 
So maybe we can implement `is_impure` property by return 
`Function::arity().num_args == 0 && Function::arity().is_varargs == false`  
rather than overridden for every impure function?.
   
   > There could be _pure_ functions called with zero arguments, in which case 
constant folding would not be incorrect.
   
   Besides, I don't understand the case that _pure_ functions called with zero 
arguments. You mean a function expression with varargs and input 
`call->arguments` is zero, or the `guaranteed_true_predicate` expression's 
`FoldConstants`.



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