zhiqiang-hhhh commented on code in PR #45414: URL: https://github.com/apache/doris/pull/45414#discussion_r1884109800
########## be/src/vec/exprs/vcompound_pred.h: ########## @@ -342,10 +380,9 @@ class VCompoundPred : public VectorizedFnCall { return (l_null & r_null) | (r_null & (r_null ^ a)) | (l_null & (l_null ^ b)); } - bool _all_child_is_compound_and_not_const() const { - return std::ranges::all_of(_children, [](const VExprSPtr& arg) -> bool { - return arg->is_compound_predicate() && !arg->is_constant(); - }); + bool _all_child_is_not_const() const { Review Comment: As we use this method like `!_all_child_is_not_const`, and it is equal to `_has_const_child`, so I think we should implement `` `_has_const_child`, which is more easy to understand. ########## be/src/vec/exprs/vcompound_pred.h: ########## @@ -158,12 +158,15 @@ class VCompoundPred : public VectorizedFnCall { if (_can_fast_execute && fast_execute(context, block, result_column_id)) { return Status::OK(); } - if (get_num_children() == 1 || !_all_child_is_compound_and_not_const()) { + if (get_num_children() == 1 || !_all_child_is_not_const()) { Review Comment: In what situation we will have just one child? ########## be/src/vec/exprs/vcompound_pred.h: ########## @@ -158,12 +158,15 @@ class VCompoundPred : public VectorizedFnCall { if (_can_fast_execute && fast_execute(context, block, result_column_id)) { return Status::OK(); } - if (get_num_children() == 1 || !_all_child_is_compound_and_not_const()) { + if (get_num_children() == 1 || !_all_child_is_not_const()) { Review Comment: `!_all_child_is_not_const()` is hard to understand. It is equal to `has_const_child()` ########## be/src/vec/exprs/vcompound_pred.h: ########## @@ -342,10 +380,9 @@ class VCompoundPred : public VectorizedFnCall { return (l_null & r_null) | (r_null & (r_null ^ a)) | (l_null & (l_null ^ b)); } - bool _all_child_is_compound_and_not_const() const { - return std::ranges::all_of(_children, [](const VExprSPtr& arg) -> bool { - return arg->is_compound_predicate() && !arg->is_constant(); - }); + bool _all_child_is_not_const() const { + return std::ranges::all_of( Review Comment: ``` return std::ranges::any_of(_children, [](const VExprSPtr& arg) -> bool { return arg->is_constant(); }); ``` -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org