BiteTheDDDDt commented on code in PR #38379: URL: https://github.com/apache/doris/pull/38379#discussion_r1695002830
########## be/src/pipeline/exec/olap_scan_operator.cpp: ########## @@ -218,8 +218,40 @@ Status OlapScanLocalState::_should_push_down_function_filter(vectorized::Vectori return Status::OK(); } -bool OlapScanLocalState::_should_push_down_common_expr() { - return state()->enable_common_expr_pushdown() && _storage_no_merge(); +bool OlapScanLocalState::_should_push_down_common_expr(const vectorized::VExprSPtr& expr) { + if (!state()->enable_common_expr_pushdown()) { + return false; + } + if (_storage_no_merge() && vectorized::VExpr::is_acting_on_a_slot(*expr)) { + return true; + } + const auto& children = expr->children(); + if (children.size() != 2) { Review Comment: why we need limit expr must has two child? ########## be/src/pipeline/exec/scan_operator.cpp: ########## @@ -190,8 +190,7 @@ Status ScanLocalState<Derived>::_normalize_conjuncts(RuntimeState* state) { RETURN_IF_ERROR(_normalize_predicate(conjunct->root(), conjunct.get(), new_root)); if (new_root) { conjunct->set_root(new_root); - if (_should_push_down_common_expr() && - vectorized::VExpr::is_acting_on_a_slot(*(conjunct->root()))) { + if (_should_push_down_common_expr(conjunct->root())) { Review Comment: pushing down expressions in the agg/mor table may cause more expression calculations. Have you do any benchmark? -- 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