github-actions[bot] commented on code in PR #31435: URL: https://github.com/apache/doris/pull/31435#discussion_r1502685628
########## be/src/pipeline/exec/aggregation_sink_operator.cpp: ########## @@ -788,8 +788,7 @@ Status AggSinkOperatorX::open(RuntimeState* state) { return Status::OK(); } -Status AggSinkOperatorX::sink(doris::RuntimeState* state, vectorized::Block* in_block, - SourceState source_state) { +Status AggSinkOperatorX::sink(doris::RuntimeState* state, vectorized::Block* in_block, bool eos) { Review Comment: warning: method 'sink' can be made static [readability-convert-member-functions-to-static] be/src/pipeline/exec/aggregation_sink_operator.h:336: ```diff - Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; + static Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; ``` ########## be/src/pipeline/exec/exchange_sink_operator.cpp: ########## @@ -300,8 +300,7 @@ void ExchangeSinkOperatorX::_handle_eof_channel(RuntimeState* state, ChannelPtrT static_cast<void>(channel->close(state, Status::OK())); } -Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, - SourceState source_state) { +Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, bool eos) { Review Comment: warning: method 'sink' can be made static [readability-convert-member-functions-to-static] be/src/pipeline/exec/exchange_sink_operator.h:196: ```diff - Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; + static Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; ``` ########## be/src/pipeline/exec/hashjoin_build_sink.cpp: ########## @@ -427,7 +427,7 @@ Status HashJoinBuildSinkOperatorX::open(RuntimeState* state) { } Status HashJoinBuildSinkOperatorX::sink(RuntimeState* state, vectorized::Block* in_block, Review Comment: warning: function 'sink' has cognitive complexity of 92 (threshold 50) [readability-function-cognitive-complexity] ```cpp Status HashJoinBuildSinkOperatorX::sink(RuntimeState* state, vectorized::Block* in_block, ^ ``` <details> <summary>Additional context</summary> **be/src/pipeline/exec/hashjoin_build_sink.cpp:434:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (local_state._should_build_hash_table) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:439:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (local_state._build_side_mutable_block.empty()) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:444:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(local_state._do_evaluate(tmp_build_block, local_state._build_expr_ctxs, ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:444:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state._do_evaluate(tmp_build_block, local_state._build_expr_ctxs, ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:451:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (in_block->rows() != 0) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:453:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(local_state._do_evaluate(*in_block, local_state._build_expr_ctxs, ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:453:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state._do_evaluate(*in_block, local_state._build_expr_ctxs, ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:458:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(local_state._build_side_mutable_block.merge(*in_block)); ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:458:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state._build_side_mutable_block.merge(*in_block)); ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:461:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (local_state._build_side_mutable_block.rows() > ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:471:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (local_state._should_build_hash_table && eos) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:471:** +1 ```cpp if (local_state._should_build_hash_table && eos) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:478:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_ERROR(vectorized::process_runtime_filter_build( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:478:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(vectorized::process_runtime_filter_build( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:481:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:481:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:483:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (_shared_hashtable_controller) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:491:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (local_state._runtime_filter_slots) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:500:** +1, nesting level increased to 1 ```cpp } else if (!local_state._should_build_hash_table) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:505:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (!_shared_hash_table_context->status.ok()) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:533:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (!_shared_hash_table_context->runtime_filters.empty()) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:536:** nesting level increased to 3 ```cpp [&](std::monostate&) -> Status { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:540:** nesting level increased to 3 ```cpp [&](auto&& arg) -> Status { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:541:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp if (local_state._runtime_filters.empty()) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:549:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state._runtime_filter_slots->init( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:549:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp RETURN_IF_ERROR(local_state._runtime_filter_slots->init( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:551:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:551:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:554:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state._runtime_filter_slots->publish(true)); ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:554:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp RETURN_IF_ERROR(local_state._runtime_filter_slots->publish(true)); ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:558:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(ret); ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:558:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(ret); ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:562:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (eos) { ^ ``` **be/src/pipeline/exec/hashjoin_build_sink.cpp:566:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (local_state._shared_state->build_block && ^ ``` </details> ########## be/src/pipeline/exec/exchange_sink_operator.cpp: ########## @@ -300,8 +300,7 @@ static_cast<void>(channel->close(state, Status::OK())); } -Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, - SourceState source_state) { +Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, bool eos) { Review Comment: warning: function 'sink' exceeds recommended size/complexity thresholds [readability-function-size] ```cpp Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, bool eos) { ^ ``` <details> <summary>Additional context</summary> **be/src/pipeline/exec/exchange_sink_operator.cpp:302:** 124 lines including whitespace and comments (threshold 80) ```cpp Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, bool eos) { ^ ``` </details> ########## be/src/pipeline/exec/multi_cast_data_stream_sink.h: ########## @@ -73,15 +73,14 @@ class MultiCastDataStreamSinkOperatorX final _sink(sink) {} ~MultiCastDataStreamSinkOperatorX() override = default; - Status sink(RuntimeState* state, vectorized::Block* in_block, - SourceState source_state) override { + Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override { Review Comment: warning: method 'sink' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override { ``` ########## be/src/pipeline/exec/sort_source_operator.cpp: ########## @@ -32,17 +32,12 @@ SortSourceOperatorX::SortSourceOperatorX(ObjectPool* pool, const TPlanNode& tnod const DescriptorTbl& descs) : OperatorX<SortLocalState>(pool, tnode, operator_id, descs) {} -Status SortSourceOperatorX::get_block(RuntimeState* state, vectorized::Block* block, - SourceState& source_state) { +Status SortSourceOperatorX::get_block(RuntimeState* state, vectorized::Block* block, bool* eos) { Review Comment: warning: method 'get_block' can be made static [readability-convert-member-functions-to-static] be/src/pipeline/exec/sort_source_operator.h:62: ```diff - Status get_block(RuntimeState* state, vectorized::Block* block, bool* eos) override; + static Status get_block(RuntimeState* state, vectorized::Block* block, bool* eos) override; ``` ########## be/src/pipeline/exec/aggregation_source_operator.cpp: ########## @@ -531,15 +526,14 @@ AggSourceOperatorX::AggSourceOperatorX(ObjectPool* pool, const TPlanNode& tnode, _needs_finalize(tnode.agg_node.need_finalize), _without_key(tnode.agg_node.grouping_exprs.empty()) {} -Status AggSourceOperatorX::get_block(RuntimeState* state, vectorized::Block* block, - SourceState& source_state) { +Status AggSourceOperatorX::get_block(RuntimeState* state, vectorized::Block* block, bool* eos) { Review Comment: warning: method 'get_block' can be made static [readability-convert-member-functions-to-static] be/src/pipeline/exec/aggregation_source_operator.h:122: ```diff - Status get_block(RuntimeState* state, vectorized::Block* block, bool* eos) override; + static Status get_block(RuntimeState* state, vectorized::Block* block, bool* eos) override; ``` ########## be/src/pipeline/exec/hashjoin_probe_operator.cpp: ########## @@ -417,7 +413,7 @@ Status HashJoinProbeLocalState::filter_data_and_build_output(RuntimeState* state, vectorized::Block* output_block, - SourceState& source_state, + bool* eos, Review Comment: warning: pointer parameter 'eos' can be pointer to const [readability-non-const-parameter] be/src/pipeline/exec/hashjoin_probe_operator.h:82: ```diff - bool* eos, vectorized::Block* temp_block, + const bool* eos, vectorized::Block* temp_block, ``` ```suggestion const bool* eos, ``` ########## be/src/pipeline/exec/datagen_operator.cpp: ########## @@ -74,19 +74,16 @@ Status DataGenSourceOperatorX::prepare(RuntimeState* state) { return Status::OK(); } -Status DataGenSourceOperatorX::get_block(RuntimeState* state, vectorized::Block* block, - SourceState& source_state) { +Status DataGenSourceOperatorX::get_block(RuntimeState* state, vectorized::Block* block, bool* eos) { Review Comment: warning: method 'get_block' can be made static [readability-convert-member-functions-to-static] be/src/pipeline/exec/datagen_operator.h:75: ```diff - Status get_block(RuntimeState* state, vectorized::Block* block, bool* eos) override; + static Status get_block(RuntimeState* state, vectorized::Block* block, bool* eos) override; ``` ########## be/src/pipeline/exec/jdbc_table_sink_operator.cpp: ########## @@ -55,11 +55,10 @@ Status JdbcTableSinkOperatorX::open(RuntimeState* state) { return Status::OK(); } -Status JdbcTableSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, - SourceState source_state) { +Status JdbcTableSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, bool eos) { Review Comment: warning: method 'sink' can be made static [readability-convert-member-functions-to-static] be/src/pipeline/exec/jdbc_table_sink_operator.h:54: ```diff - Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; + static Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; ``` ########## be/src/pipeline/exec/olap_table_sink_v2_operator.h: ########## @@ -93,12 +93,11 @@ class OlapTableSinkV2OperatorX final : public DataSinkOperatorX<OlapTableSinkV2L return vectorized::VExpr::open(_output_vexpr_ctxs, state); } - Status sink(RuntimeState* state, vectorized::Block* in_block, - SourceState source_state) override { + Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override { Review Comment: warning: method 'sink' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override { ``` ########## be/src/pipeline/exec/hashjoin_probe_operator.cpp: ########## @@ -232,13 +232,13 @@ HashJoinProbeOperatorX::HashJoinProbeOperatorX(ObjectPool* pool, const TPlanNode : std::vector<TExpr> {}) {} Status HashJoinProbeOperatorX::pull(doris::RuntimeState* state, vectorized::Block* output_block, Review Comment: warning: function 'pull' has cognitive complexity of 53 (threshold 50) [readability-function-cognitive-complexity] ```cpp Status HashJoinProbeOperatorX::pull(doris::RuntimeState* state, vectorized::Block* output_block, ^ ``` <details> <summary>Additional context</summary> **be/src/pipeline/exec/hashjoin_probe_operator.cpp:238:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (local_state._shared_state->short_circuit_for_probe) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:245:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (local_state._shared_state->empty_right_table_need_probe_dispose) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:250:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (local_state._probe_eos && block_rows == 0) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:250:** +1 ```cpp if (local_state._probe_eos && block_rows == 0) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:274:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (_is_outer_join) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:287:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_ERROR(local_state.filter_data_and_build_output(state, output_block, eos, ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:287:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(local_state.filter_data_and_build_output(state, output_block, eos, ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:300:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (local_state._probe_index < local_state._probe_block.rows()) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:302:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_CATCH_EXCEPTION({ ^ ``` **be/src/common/exception.h:89:** expanded from macro 'RETURN_IF_CATCH_EXCEPTION' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:304:** nesting level increased to 3 ```cpp [&](auto&& arg, auto&& process_hashtable_ctx, auto need_null_map_for_probe, ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:307:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp if constexpr (!std::is_same_v<HashTableProbeType, std::monostate>) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:309:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp if constexpr (!std::is_same_v<HashTableCtxType, std::monostate>) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:321:** +1, nesting level increased to 5 ```cpp } else { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:324:** +1, nesting level increased to 4 ```cpp } else { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:302:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_CATCH_EXCEPTION({ ^ ``` **be/src/common/exception.h:94:** expanded from macro 'RETURN_IF_CATCH_EXCEPTION' ```cpp } catch (const doris::Exception& e) { \ ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:302:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_CATCH_EXCEPTION({ ^ ``` **be/src/common/exception.h:95:** expanded from macro 'RETURN_IF_CATCH_EXCEPTION' ```cpp if (e.code() == doris::ErrorCode::MEM_ALLOC_FAILED) { \ ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:333:** +1, nesting level increased to 1 ```cpp } else if (local_state._probe_eos) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:334:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (_is_right_semi_anti || (_is_outer_join && _join_op != TJoinOp::LEFT_OUTER_JOIN)) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:336:** nesting level increased to 3 ```cpp [&](auto&& arg, auto&& process_hashtable_ctx) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:338:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp if constexpr (!std::is_same_v<HashTableProbeType, std::monostate>) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:340:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp if constexpr (!std::is_same_v<HashTableCtxType, std::monostate>) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:343:** +1, nesting level increased to 5 ```cpp } else { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:346:** +1, nesting level increased to 4 ```cpp } else { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:352:** +1, nesting level increased to 2 ```cpp } else { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:356:** +1, nesting level increased to 1 ```cpp } else { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:359:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (!st) { ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:363:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/hashjoin_probe_operator.cpp:363:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` </details> ########## be/src/pipeline/exec/table_function_operator.h: ########## @@ -92,16 +91,14 @@ class TableFunctionOperatorX final : public StatefulOperatorX<TableFunctionLocal bool need_more_input_data(RuntimeState* state) const override { auto& local_state = state->get_local_state(operator_id())->cast<TableFunctionLocalState>(); - return !local_state._child_block->rows() && - local_state._child_source_state != SourceState::FINISHED; + return !local_state._child_block->rows() && !local_state._child_eos; } DataDistribution required_data_distribution() const override { return {ExchangeType::PASSTHROUGH}; } - Status push(RuntimeState* state, vectorized::Block* input_block, - SourceState source_state) const override { + Status push(RuntimeState* state, vectorized::Block* input_block, bool eos) const override { Review Comment: warning: method 'push' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status push(RuntimeState* state, vectorized::Block* input_block, bool eos) override { ``` ########## be/src/pipeline/exec/repeat_operator.cpp: ########## @@ -179,10 +177,9 @@ Status RepeatLocalState::get_repeated_block(vectorized::Block* child_block, int return Status::OK(); } -Status RepeatOperatorX::push(RuntimeState* state, vectorized::Block* input_block, - SourceState source_state) const { +Status RepeatOperatorX::push(RuntimeState* state, vectorized::Block* input_block, bool eos) const { Review Comment: warning: method 'push' can be made static [readability-convert-member-functions-to-static] ```suggestion Status RepeatOperatorX::push(RuntimeState* state, vectorized::Block* input_block, bool eos) { ``` be/src/pipeline/exec/repeat_operator.h:84: ```diff - Status push(RuntimeState* state, vectorized::Block* input_block, bool eos) const override; + static Status push(RuntimeState* state, vectorized::Block* input_block, bool eos) override; ``` ########## be/src/pipeline/exec/result_file_sink_operator.cpp: ########## @@ -262,12 +262,11 @@ void ResultFileSinkLocalState::_handle_eof_channel(RuntimeState* state, ChannelP static_cast<void>(channel->close(state, Status::OK())); } -Status ResultFileSinkOperatorX::sink(RuntimeState* state, vectorized::Block* in_block, - SourceState source_state) { +Status ResultFileSinkOperatorX::sink(RuntimeState* state, vectorized::Block* in_block, bool eos) { Review Comment: warning: method 'sink' can be made static [readability-convert-member-functions-to-static] be/src/pipeline/exec/result_file_sink_operator.h:102: ```diff - Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; + static Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override; ``` ########## be/src/pipeline/exec/table_function_operator.h: ########## @@ -114,11 +111,10 @@ return Status::OK(); } - Status pull(RuntimeState* state, vectorized::Block* output_block, - SourceState& source_state) const override { + Status pull(RuntimeState* state, vectorized::Block* output_block, bool* eos) const override { Review Comment: warning: method 'pull' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status pull(RuntimeState* state, vectorized::Block* output_block, bool* eos) override { ``` ########## be/src/pipeline/exec/select_operator.h: ########## @@ -59,13 +59,13 @@ class SelectOperatorX final : public StreamingOperatorX<SelectLocalState> { const DescriptorTbl& descs) : StreamingOperatorX<SelectLocalState>(pool, tnode, operator_id, descs) {} - Status pull(RuntimeState* state, vectorized::Block* block, SourceState& source_state) override { + Status pull(RuntimeState* state, vectorized::Block* block, bool* eos) override { Review Comment: warning: method 'pull' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status pull(RuntimeState* state, vectorized::Block* block, bool* eos) override { ``` ########## be/src/pipeline/exec/set_source_operator.h: ########## @@ -102,7 +101,7 @@ class SetSourceOperatorX final : public OperatorX<SetSourceLocalState<is_interse template <typename HashTableContext> Status _get_data_in_hashtable(SetSourceLocalState<is_intersect>& local_state, HashTableContext& hash_table_ctx, vectorized::Block* output_block, - const int batch_size, SourceState& source_state); + const int batch_size, bool* eos); Review Comment: warning: parameter 'batch_size' 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 int batch_size, bool* eos); ``` ########## be/src/pipeline/exec/exchange_sink_operator.cpp: ########## @@ -300,8 +300,7 @@ static_cast<void>(channel->close(state, Status::OK())); } -Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, - SourceState source_state) { +Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, bool eos) { Review Comment: warning: function 'sink' has cognitive complexity of 123 (threshold 50) [readability-function-cognitive-complexity] ```cpp Status ExchangeSinkOperatorX::sink(RuntimeState* state, vectorized::Block* block, bool eos) { ^ ``` <details> <summary>Additional context</summary> **be/src/pipeline/exec/exchange_sink_operator.cpp:315:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (all_receiver_eof) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:319:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (_part_type == TPartitionType::UNPARTITIONED || local_state.channels.size() == 1) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:323:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (local_state.only_local_exchange) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:324:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (!block->empty()) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:333:** +1, nesting level increased to 2 ```cpp } else { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:335:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(local_state.get_next_available_buffer(&block_holder)); ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:335:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state.get_next_available_buffer(&block_holder)); ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:339:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(local_state._serializer.next_serialized_block( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:339:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state._serializer.next_serialized_block( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:342:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (serialized) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:344:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp if (!cur_block.empty()) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:348:** +1, nesting level increased to 4 ```cpp } else { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:369:** +1, nesting level increased to 1 ```cpp } else if (_part_type == TPartitionType::RANDOM) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:373:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (!current_channel->is_receiver_eof()) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:375:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (current_channel->is_local()) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:377:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:391:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:377:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:392:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp if (status.is<ErrorCode::END_OF_FILE>()) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:377:** +1, nesting level increased to 5 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:394:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp } else { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:377:** +6, including nesting penalty of 5, nesting level increased to 6 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:395:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp RETURN_IF_ERROR(status); \ ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:377:** +7, including nesting penalty of 6, nesting level increased to 7 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:395:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp RETURN_IF_ERROR(status); \ ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:378:** +1, nesting level increased to 3 ```cpp } else { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:380:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(local_state._serializer.serialize_block( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:380:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp RETURN_IF_ERROR(local_state._serializer.serialize_block( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:384:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:391:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:384:** +5, including nesting penalty of 4, nesting level increased to 5 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:392:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp if (status.is<ErrorCode::END_OF_FILE>()) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:384:** +1, nesting level increased to 5 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:394:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp } else { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:384:** +6, including nesting penalty of 5, nesting level increased to 6 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:395:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp RETURN_IF_ERROR(status); \ ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:384:** +7, including nesting penalty of 6, nesting level increased to 7 ```cpp HANDLE_CHANNEL_STATUS(state, current_channel, status); ^ ``` **be/src/vec/sink/vdata_stream_sender.h:395:** expanded from macro 'HANDLE_CHANNEL_STATUS' ```cpp RETURN_IF_ERROR(status); \ ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:390:** +1, nesting level increased to 1 ```cpp } else if (_part_type == TPartitionType::HASH_PARTITIONED || ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:395:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:395:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:398:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp if (_part_type == TPartitionType::HASH_PARTITIONED) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:399:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(channel_add_rows( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:399:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(channel_add_rows( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:402:** +1, nesting level increased to 2 ```cpp } else { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:403:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp RETURN_IF_ERROR(channel_add_rows( ^ ``` **be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR' ```cpp do { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:403:** +4, including nesting penalty of 3, nesting level increased to 4 ```cpp RETURN_IF_ERROR(channel_add_rows( ^ ``` **be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR' ```cpp if (UNLIKELY(!_status_.ok())) { \ ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:407:** +1, nesting level increased to 1 ```cpp } else { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:414:** +1, including nesting penalty of 0, nesting level increased to 1 ```cpp if (eos) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:416:** +2, including nesting penalty of 1, nesting level increased to 2 ```cpp for (int i = 0; i < local_state.channels.size(); ++i) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:418:** +3, including nesting penalty of 2, nesting level increased to 3 ```cpp if (!st.ok() && final_st.ok()) { ^ ``` **be/src/pipeline/exec/exchange_sink_operator.cpp:418:** +1 ```cpp if (!st.ok() && final_st.ok()) { ^ ``` </details> ########## be/src/pipeline/exec/streaming_aggregation_operator.cpp: ########## @@ -1264,26 +1262,24 @@ Status StreamingAggLocalState::close(RuntimeState* state) { return Base::close(state); } -Status StreamingAggOperatorX::pull(RuntimeState* state, vectorized::Block* block, - SourceState& source_state) const { +Status StreamingAggOperatorX::pull(RuntimeState* state, vectorized::Block* block, bool* eos) const { Review Comment: warning: method 'pull' can be made static [readability-convert-member-functions-to-static] ```suggestion Status StreamingAggOperatorX::pull(RuntimeState* state, vectorized::Block* block, bool* eos) { ``` be/src/pipeline/exec/streaming_aggregation_operator.h:194: ```diff - Status pull(RuntimeState* state, vectorized::Block* block, bool* eos) const override; + static Status pull(RuntimeState* state, vectorized::Block* block, bool* eos) override; ``` ########## be/src/pipeline/exec/olap_table_sink_operator.h: ########## @@ -91,12 +91,11 @@ class OlapTableSinkOperatorX final : public DataSinkOperatorX<OlapTableSinkLocal RETURN_IF_ERROR(Base::open(state)); return vectorized::VExpr::open(_output_vexpr_ctxs, state); } - Status sink(RuntimeState* state, vectorized::Block* in_block, - SourceState source_state) override { + Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override { Review Comment: warning: method 'sink' can be made static [readability-convert-member-functions-to-static] ```suggestion static Status sink(RuntimeState* state, vectorized::Block* in_block, bool eos) override { ``` -- 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