Copilot commented on code in PR #60147:
URL: https://github.com/apache/doris/pull/60147#discussion_r2715902326


##########
be/src/pipeline/exec/scan_operator.cpp:
##########
@@ -364,7 +364,15 @@ Status 
ScanLocalState<Derived>::_normalize_predicate(vectorized::VExprContext* c
                     auto expr = root->is_rf_wrapper() ? root->get_impl() : 
root;
                     {
                         Defer attach_defer = [&]() {
-                            if (pdt != PushDownType::UNACCEPTABLE && 
root->is_rf_wrapper()) {
+                            // eos is true means triggered short-circuit in 
predicate pushdown, predicates may empty

Review Comment:
   Grammar issue in comment: "predicates may empty" should be "predicates may 
be empty".
   ```suggestion
                               // eos is true means triggered short-circuit in 
predicate pushdown, predicates may be empty
   ```



##########
be/src/pipeline/exec/scan_operator.cpp:
##########
@@ -364,7 +364,15 @@ Status 
ScanLocalState<Derived>::_normalize_predicate(vectorized::VExprContext* c
                     auto expr = root->is_rf_wrapper() ? root->get_impl() : 
root;
                     {
                         Defer attach_defer = [&]() {
-                            if (pdt != PushDownType::UNACCEPTABLE && 
root->is_rf_wrapper()) {
+                            // eos is true means triggered short-circuit in 
predicate pushdown, predicates may empty
+                            if (pdt != PushDownType::UNACCEPTABLE && 
root->is_rf_wrapper() &&
+                                !_eos) {
+                                if 
(_slot_id_to_predicates[slot->id()].empty()) {
+                                    return Status::InternalError(
+                                            "No predicate generated for 
runtime filter "
+                                            "from expr: {}",
+                                            root->debug_string());

Review Comment:
   Returning a Status object from a Defer lambda is incorrect. The Defer 
destructor invokes the closure without expecting or handling a return value. 
The returned Status::InternalError will be silently discarded. If you need 
error handling here, consider logging the error or setting a status variable 
captured by the lambda that can be checked after the defer block executes.
   ```suggestion
                                       status = Status::InternalError(
                                               "No predicate generated for 
runtime filter "
                                               "from expr: {}",
                                               root->debug_string());
                                       return;
   ```



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to