github-actions[bot] commented on code in PR #27886:
URL: https://github.com/apache/doris/pull/27886#discussion_r1415148697
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp:
##########
@@ -249,40 +250,44 @@
return Status::OK();
}
-Status PipelineXFragmentContext::_plan_local_shuffle() {
+Status PipelineXFragmentContext::_plan_local_shuffle(
+ int num_buckets, const std::map<int, int>& bucket_seq_to_instance_idx)
{
for (int pip_idx = _pipelines.size() - 1; pip_idx >= 0; pip_idx--) {
- auto& children = _pipelines[pip_idx]->children();
- if (children.empty()) {
- _pipelines[pip_idx]->init_need_to_local_shuffle_by_source();
- } else if (children.size() == 1) {
-
_pipelines[pip_idx]->set_need_to_local_shuffle(children[0]->need_to_local_shuffle());
- }
+ _pipelines[pip_idx]->init_need_to_local_shuffle_by_source();
- int idx = 0;
- bool do_local_exchange = false;
- do {
- auto& ops = _pipelines[pip_idx]->operator_xs();
- do_local_exchange = false;
- for (; idx < ops.size();) {
- if (ops[idx]->get_local_exchange_type() != ExchangeType::NOOP)
{
- RETURN_IF_ERROR(_add_local_exchange(
- idx, ops[idx]->node_id(),
_runtime_state->obj_pool(),
- _pipelines[pip_idx],
ops[idx]->get_local_shuffle_exprs(),
- ops[idx]->get_local_exchange_type(),
&do_local_exchange));
- }
- if (do_local_exchange) {
- idx = 2;
- break;
- }
- idx++;
+ RETURN_IF_ERROR(_plan_local_shuffle(num_buckets, pip_idx,
_pipelines[pip_idx],
+ bucket_seq_to_instance_idx));
+ }
+ return Status::OK();
+}
+
+Status PipelineXFragmentContext::_plan_local_shuffle(
Review Comment:
warning: method '_plan_local_shuffle' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status PipelineXFragmentContext::_plan_local_shuffle(
```
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.h:
##########
@@ -125,9 +125,10 @@ class PipelineXFragmentContext : public
PipelineFragmentContext {
private:
void _close_fragment_instance() override;
Status _build_pipeline_tasks(const doris::TPipelineFragmentParams&
request) override;
- Status _add_local_exchange(int idx, int node_id, ObjectPool* pool,
PipelinePtr cur_pipe,
- const std::vector<TExpr>& texprs, ExchangeType
exchange_type,
- bool* do_local_exchange);
+ Status _add_local_exchange(int pip_idx, int idx, int node_id, ObjectPool*
pool,
+ PipelinePtr cur_pipe, const std::vector<TExpr>&
texprs,
+ ExchangeType exchange_type, bool*
do_local_exchange, int num_buckets,
+ const std::map<int, int>&
bucket_seq_to_instance_idx);
Review Comment:
warning: parameter 10 is const-qualified in the function declaration;
const-qualification of parameters only has an effect in function definitions
[readability-avoid-const-params-in-decls]
```suggestion
std::map<int, int>&
bucket_seq_to_instance_idx);
```
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp:
##########
@@ -634,16 +639,17 @@
return Status::OK();
}
-Status PipelineXFragmentContext::_add_local_exchange(int idx, int node_id,
ObjectPool* pool,
- PipelinePtr cur_pipe,
- const std::vector<TExpr>&
texprs,
- ExchangeType
exchange_type,
- bool* do_local_exchange) {
+Status PipelineXFragmentContext::_add_local_exchange(
Review Comment:
warning: function '_add_local_exchange' exceeds recommended size/complexity
thresholds [readability-function-size]
```cpp
Status PipelineXFragmentContext::_add_local_exchange(
^
```
<details>
<summary>Additional context</summary>
**be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp:641:** 98 lines
including whitespace and comments (threshold 80)
```cpp
Status PipelineXFragmentContext::_add_local_exchange(
^
```
</details>
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.h:
##########
@@ -153,7 +154,10 @@
const TPipelineFragmentParams& params, const
RowDescriptor& row_desc,
RuntimeState* state, DescriptorTbl& desc_tbl,
PipelineId cur_pipeline_id);
- Status _plan_local_shuffle();
+ Status _plan_local_shuffle(int num_buckets,
+ const std::map<int, int>&
bucket_seq_to_instance_idx);
Review Comment:
warning: parameter 2 is const-qualified in the function declaration;
const-qualification of parameters only has an effect in function definitions
[readability-avoid-const-params-in-decls]
```suggestion
std::map<int, int>&
bucket_seq_to_instance_idx);
```
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp:
##########
@@ -249,40 +250,44 @@ Status PipelineXFragmentContext::prepare(const
doris::TPipelineFragmentParams& r
return Status::OK();
}
-Status PipelineXFragmentContext::_plan_local_shuffle() {
+Status PipelineXFragmentContext::_plan_local_shuffle(
Review Comment:
warning: method '_plan_local_shuffle' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status PipelineXFragmentContext::_plan_local_shuffle(
```
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.h:
##########
@@ -153,7 +154,10 @@
const TPipelineFragmentParams& params, const
RowDescriptor& row_desc,
RuntimeState* state, DescriptorTbl& desc_tbl,
PipelineId cur_pipeline_id);
- Status _plan_local_shuffle();
+ Status _plan_local_shuffle(int num_buckets,
+ const std::map<int, int>&
bucket_seq_to_instance_idx);
+ Status _plan_local_shuffle(int num_buckets, int pip_idx, PipelinePtr pip,
+ const std::map<int, int>&
bucket_seq_to_instance_idx);
Review Comment:
warning: parameter 4 is const-qualified in the function declaration;
const-qualification of parameters only has an effect in function definitions
[readability-avoid-const-params-in-decls]
```suggestion
std::map<int, int>&
bucket_seq_to_instance_idx);
```
--
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
