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


##########
be/src/runtime/runtime_state.h:
##########
@@ -586,6 +586,11 @@ class RuntimeState {
                _query_options.enable_distinct_streaming_agg_force_passthrough;
     }
 
+    bool enable_local_exchange_before_agg() const {
+        return _query_options.__isset.enable_local_exchange_before_agg &&
+               _query_options.enable_local_exchange_before_agg;
+    }

Review Comment:
   `RuntimeState::enable_local_exchange_before_agg()` currently requires 
`__isset.enable_local_exchange_before_agg` to be true. But 
`enable_local_exchange_before_agg` is defined in thrift with default `= true`, 
so when the option is not explicitly set, `__isset` will be false even though 
the effective default should be enabled. This changes the default behavior to 
“disabled”, which can incorrectly trigger the new “skip local exchange before 
agg” paths by default.



##########
be/src/exec/operator/aggregation_sink_operator.cpp:
##########
@@ -752,6 +752,7 @@ Status AggSinkOperatorX::init(const TPlanNode& tnode, 
RuntimeState* state) {
                 tnode.agg_node.__isset.agg_sort_infos ? 
tnode.agg_node.agg_sort_infos[i] : dummy,
                 tnode.agg_node.grouping_exprs.empty(), false, &evaluator));
         _aggregate_evaluators.push_back(evaluator);
+        _is_merge |= evaluator->is_merge();
     }

Review Comment:
   `_is_merge` is updated via `|=` inside the init loop, but it isn’t 
explicitly reset in `init()`. This makes the result depend on prior state (and 
in BE_TEST builds the default ctor doesn’t initialize `_is_merge`), which can 
lead to incorrect `_is_merge` and executor selection. Reset `_is_merge` (and 
ideally other per-init fields) before accumulating.



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