github-actions[bot] commented on code in PR #28692:
URL: https://github.com/apache/doris/pull/28692#discussion_r1432196542
##########
be/src/pipeline/exec/hashjoin_probe_operator.h:
##########
@@ -163,17 +163,17 @@ class HashJoinProbeOperatorX final : public
JoinProbeOperatorX<HashJoinProbeLoca
SourceState& source_state) const override;
bool need_more_input_data(RuntimeState* state) const override;
- std::vector<TExpr> get_local_shuffle_exprs() const override { return
_partition_exprs; }
- ExchangeType get_local_exchange_type() const override {
+ DataDistribution get_local_exchange_type() const override {
Review Comment:
warning: method 'get_local_exchange_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static DataDistribution get_local_exchange_type() override {
```
##########
be/src/pipeline/exec/assert_num_rows_operator.h:
##########
@@ -57,7 +57,9 @@ class AssertNumRowsOperatorX final : public
StreamingOperatorX<AssertNumRowsLoca
[[nodiscard]] bool is_source() const override { return false; }
- ExchangeType get_local_exchange_type() const override { return
ExchangeType::PASSTHROUGH; }
+ DataDistribution get_local_exchange_type() const override {
Review Comment:
warning: method 'get_local_exchange_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static DataDistribution get_local_exchange_type() override {
```
##########
be/src/pipeline/exec/partition_sort_sink_operator.h:
##########
@@ -105,11 +105,11 @@ class PartitionSortSinkOperatorX final : public
DataSinkOperatorX<PartitionSortS
Status open(RuntimeState* state) override;
Status sink(RuntimeState* state, vectorized::Block* in_block,
SourceState source_state) override;
- ExchangeType get_local_exchange_type() const override {
+ DataDistribution get_local_exchange_type() const override {
Review Comment:
warning: method 'get_local_exchange_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static DataDistribution get_local_exchange_type() override {
```
##########
be/src/pipeline/exec/analytic_sink_operator.h:
##########
@@ -107,14 +107,15 @@ class AnalyticSinkOperatorX final : public
DataSinkOperatorX<AnalyticSinkLocalSt
Status sink(RuntimeState* state, vectorized::Block* in_block,
SourceState source_state) override;
- std::vector<TExpr> get_local_shuffle_exprs() const override { return
_partition_exprs; }
- ExchangeType get_local_exchange_type() const override {
+ DataDistribution get_local_exchange_type() const override {
Review Comment:
warning: method 'get_local_exchange_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static DataDistribution get_local_exchange_type() override {
```
##########
be/src/pipeline/exec/exchange_source_operator.h:
##########
@@ -116,8 +116,11 @@ class ExchangeSourceOperatorX final : public
OperatorX<ExchangeLocalState> {
return _sub_plan_query_statistics_recvr;
}
- bool need_to_local_shuffle() const override {
- return !_is_hash_partition ||
OperatorX<ExchangeLocalState>::ignore_data_distribution();
+ DataDistribution get_local_exchange_type() const override {
Review Comment:
warning: method 'get_local_exchange_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static DataDistribution get_local_exchange_type() override {
```
##########
be/src/pipeline/exec/streaming_aggregation_sink_operator.h:
##########
@@ -120,7 +120,9 @@ class StreamingAggSinkOperatorX final : public
AggSinkOperatorX<StreamingAggSink
Status init(const TPlanNode& tnode, RuntimeState* state) override;
Status sink(RuntimeState* state, vectorized::Block* in_block,
SourceState source_state) override;
- ExchangeType get_local_exchange_type() const override { return
ExchangeType::PASSTHROUGH; }
+ DataDistribution get_local_exchange_type() const override {
Review Comment:
warning: method 'get_local_exchange_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static DataDistribution get_local_exchange_type() override {
```
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.h:
##########
@@ -156,7 +157,8 @@
Status _plan_local_exchange(int num_buckets,
const std::map<int, int>&
bucket_seq_to_instance_idx);
Status _plan_local_exchange(int num_buckets, int pip_idx, PipelinePtr pip,
- const std::map<int, int>&
bucket_seq_to_instance_idx);
+ 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,
```
##########
be/src/pipeline/pipeline.h:
##########
@@ -127,29 +122,40 @@ class Pipeline : public
std::enable_shared_from_this<Pipeline> {
return _collect_query_statistics_with_every_batch;
}
- bool need_to_local_shuffle() const { return _need_to_local_shuffle; }
- void set_need_to_local_shuffle(bool need_to_local_shuffle) {
- _need_to_local_shuffle = need_to_local_shuffle;
+ bool need_to_local_shuffle(const DataDistribution
target_data_distribution) const {
Review Comment:
warning: method 'need_to_local_shuffle' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static bool need_to_local_shuffle(const DataDistribution
target_data_distribution) {
```
##########
be/src/pipeline/exec/nested_loop_join_probe_operator.h:
##########
@@ -227,11 +227,11 @@ class NestedLoopJoinProbeOperatorX final
return _old_version_flag ? _row_descriptor : *_intermediate_row_desc;
}
- ExchangeType get_local_exchange_type() const override {
+ DataDistribution get_local_exchange_type() const override {
Review Comment:
warning: method 'get_local_exchange_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static DataDistribution get_local_exchange_type() override {
```
##########
be/src/pipeline/pipeline_x/local_exchange/local_exchange_source_operator.h:
##########
@@ -81,8 +83,13 @@ class LocalExchangeSourceOperatorX final : public
OperatorX<LocalExchangeSourceL
bool is_source() const override { return true; }
+ // If input data distribution is ignored by this fragment, this first
local exchange source in this fragment will re-assign all data.
+ bool ignore_data_distribution() const override { return false; }
Review Comment:
warning: method 'ignore_data_distribution' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static bool ignore_data_distribution() override { return false; }
```
##########
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.h:
##########
@@ -123,11 +123,12 @@ class PipelineXFragmentContext : public
PipelineFragmentContext {
void _close_fragment_instance() override;
Status _build_pipeline_tasks(const doris::TPipelineFragmentParams&
request) override;
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);
- void _inherit_pipeline_properties(ExchangeType exchange_type, PipelinePtr
pipe_with_source,
- PipelinePtr pipe_with_sink);
+ PipelinePtr cur_pipe, DataDistribution
data_distribution,
+ bool* do_local_exchange, int num_buckets,
+ const std::map<int, int>&
bucket_seq_to_instance_idx,
Review Comment:
warning: parameter 9 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: [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]
