github-actions[bot] commented on code in PR #31296:
URL: https://github.com/apache/doris/pull/31296#discussion_r1500985978
##########
be/src/olap/rowset/segment_v2/column_reader.cpp:
##########
@@ -395,19 +396,20 @@ void ColumnReader::_parse_zone_map(const ZoneMapPB&
zone_map, WrapperField* min_
}
}
-void ColumnReader::_parse_zone_map_skip_null(const ZoneMapPB& zone_map,
- WrapperField* min_value_container,
- WrapperField*
max_value_container) const {
+Status ColumnReader::_parse_zone_map_skip_null(const ZoneMapPB& zone_map,
Review Comment:
warning: method '_parse_zone_map_skip_null' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status ColumnReader::_parse_zone_map_skip_null(const ZoneMapPB&
zone_map,
```
be/src/olap/rowset/segment_v2/column_reader.cpp:400:
```diff
- WrapperField*
max_value_container) const {
+ WrapperField*
max_value_container) {
```
##########
be/src/io/fs/buffered_reader.h:
##########
@@ -409,13 +409,14 @@ class PrefetchBufferedReader : public io::FileReader {
size_t get_buffer_offset(int64_t position) const {
return (position / s_max_pre_buffer_size) * s_max_pre_buffer_size;
}
- void reset_all_buffer(size_t position) {
+ Status reset_all_buffer(size_t position) {
Review Comment:
warning: method 'reset_all_buffer' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status reset_all_buffer(size_t position) {
```
##########
be/src/pipeline/task_scheduler.cpp:
##########
@@ -248,7 +249,7 @@
task->fragment_context()->close_a_pipeline();
}
-void TaskScheduler::_do_work(size_t index) {
+Status TaskScheduler::_do_work(size_t index) {
Review Comment:
warning: function '_do_work' has cognitive complexity of 70 (threshold 50)
[readability-function-cognitive-complexity]
```cpp
Status TaskScheduler::_do_work(size_t index) {
^
```
<details>
<summary>Additional context</summary>
**be/src/pipeline/task_scheduler.cpp:253:** +1, including nesting penalty of
0, nesting level increased to 1
```cpp
while (*marker) {
^
```
**be/src/pipeline/task_scheduler.cpp:255:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
if (!task) {
^
```
**be/src/pipeline/task_scheduler.cpp:258:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
if (task->is_pipelineX() && task->is_running()) {
^
```
**be/src/pipeline/task_scheduler.cpp:258:** +1
```cpp
if (task->is_pipelineX() && task->is_running()) {
^
```
**be/src/pipeline/task_scheduler.cpp:275:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
if (state == PipelineTaskState::PENDING_FINISH) {
^
```
**be/src/pipeline/task_scheduler.cpp:276:** +1
```cpp
DCHECK(task->is_pipelineX() || !task->is_pending_finish())
^
```
**be/src/pipeline/task_scheduler.cpp:279:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
_close_task(task, canceled ? PipelineTaskState::CANCELED :
PipelineTaskState::FINISHED,
^
```
**be/src/pipeline/task_scheduler.cpp:284:** +1
```cpp
DCHECK(state != PipelineTaskState::FINISHED && state !=
PipelineTaskState::CANCELED)
^
```
**be/src/pipeline/task_scheduler.cpp:287:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
if (canceled) {
^
```
**be/src/pipeline/task_scheduler.cpp:299:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
^
```
**be/src/pipeline/task_scheduler.cpp:303:** +1
```cpp
^
```
**be/src/pipeline/task_scheduler.cpp:312:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
e
^
```
**be/src/pipeline/task_scheduler.cpp:325:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
=
^
```
**be/src/pipeline/task_scheduler.cpp:329:** +1, nesting level increased to 2
```cpp
;
^
```
**be/src/pipeline/task_scheduler.cpp:332:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
}
^
```
**be/src/pipeline/task_scheduler.cpp:338:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
^
```
**be/src/pipeline/task_scheduler.cpp:342:** +1, nesting level increased to 2
```cpp
;
^
```
**be/src/pipeline/task_scheduler.cpp:361:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
);
^
```
**be/src/pipeline/task_scheduler.cpp:370:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
));
^
```
**be/src/pipeline/task_scheduler.cpp:373:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
dy.
^
```
**be/src/pipeline/task_scheduler.cpp:377:** +1, nesting level increased to 4
```cpp
e);
^
```
**be/src/pipeline/task_scheduler.cpp:382:** +5, including nesting penalty of
4, nesting level increased to 5
```cpp
sk,
^
```
**be/src/pipeline/task_scheduler.cpp:385:** +1, nesting level increased to 3
```cpp
}
^
```
**be/src/pipeline/task_scheduler.cpp:393:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
ng.
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/task_scheduler.cpp:393:** +5, including nesting penalty of
4, nesting level increased to 5
```cpp
ng.
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/task_scheduler.cpp:399:** +2, including nesting penalty of
1, nesting level increased to 2
```cpp
();
^
```
**be/src/pipeline/task_scheduler.cpp:404:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
CY:
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/task_scheduler.cpp:404:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
CY:
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
**be/src/pipeline/task_scheduler.cpp:408:** +3, including nesting penalty of
2, nesting level increased to 3
```cpp
e);
^
```
**be/src/common/status.h:541:** expanded from macro 'RETURN_IF_ERROR'
```cpp
do { \
^
```
**be/src/pipeline/task_scheduler.cpp:408:** +4, including nesting penalty of
3, nesting level increased to 4
```cpp
e);
^
```
**be/src/common/status.h:543:** expanded from macro 'RETURN_IF_ERROR'
```cpp
if (UNLIKELY(!_status_.ok())) { \
^
```
</details>
##########
be/src/olap/rowset/segment_v2/column_reader.h:
##########
@@ -171,11 +171,12 @@ class ColumnReader {
uint64_t num_rows() const { return _num_rows; }
- void set_dict_encoding_type(DictEncodingType type) {
- static_cast<void>(_set_dict_encoding_type_once.call([&] {
+ Status set_dict_encoding_type(DictEncodingType type) {
Review Comment:
warning: method 'set_dict_encoding_type' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status set_dict_encoding_type(DictEncodingType type) {
```
##########
be/src/vec/functions/function_rpc.cpp:
##########
@@ -61,28 +62,30 @@ Status RPCFnImpl::vec_call(FunctionContext* context, Block&
block, const ColumnN
return Status::InternalError("call to rpc function {} failed: {}",
_signature,
response.status().DebugString());
}
- _convert_to_block(block, response.result(0), result);
+ RETURN_IF_ERROR(_convert_to_block(block, response.result(0), result));
return Status::OK();
}
-void RPCFnImpl::_convert_block_to_proto(Block& block, const ColumnNumbers&
arguments,
- size_t input_rows_count,
PFunctionCallRequest* request) {
+Status RPCFnImpl::_convert_block_to_proto(Block& block, const ColumnNumbers&
arguments,
Review Comment:
warning: method '_convert_block_to_proto' can be made static
[readability-convert-member-functions-to-static]
```suggestion
static Status RPCFnImpl::_convert_block_to_proto(Block& block, const
ColumnNumbers& arguments,
```
##########
be/src/vec/functions/function_rpc.cpp:
##########
@@ -61,28 +62,30 @@
return Status::InternalError("call to rpc function {} failed: {}",
_signature,
response.status().DebugString());
}
- _convert_to_block(block, response.result(0), result);
+ RETURN_IF_ERROR(_convert_to_block(block, response.result(0), result));
return Status::OK();
}
-void RPCFnImpl::_convert_block_to_proto(Block& block, const ColumnNumbers&
arguments,
- size_t input_rows_count,
PFunctionCallRequest* request) {
+Status RPCFnImpl::_convert_block_to_proto(Block& block, const ColumnNumbers&
arguments,
+ size_t input_rows_count,
PFunctionCallRequest* request) {
size_t row_count = std::min(block.rows(), input_rows_count);
for (size_t col_idx : arguments) {
PValues* arg = request->add_args();
ColumnWithTypeAndName& column = block.get_by_position(col_idx);
arg->set_has_null(column.column->has_null(row_count));
auto col = column.column->convert_to_full_column_if_const();
- static_cast<void>(column.type->get_serde()->write_column_to_pb(*col,
*arg, 0, row_count));
+ RETURN_IF_ERROR(column.type->get_serde()->write_column_to_pb(*col,
*arg, 0, row_count));
}
+ return Status::OK();
}
-void RPCFnImpl::_convert_to_block(Block& block, const PValues& result, size_t
pos) {
+Status RPCFnImpl::_convert_to_block(Block& block, const PValues& result,
size_t pos) {
Review Comment:
warning: method '_convert_to_block' can be made static
[readability-convert-member-functions-to-static]
be/src/vec/functions/function_rpc.h:57:
```diff
- Status _convert_to_block(vectorized::Block& block, const PValues&
result, size_t pos);
+ static Status _convert_to_block(vectorized::Block& block, const
PValues& result, size_t pos);
```
##########
be/src/pipeline/task_scheduler.cpp:
##########
@@ -248,7 +249,7 @@ void _close_task(PipelineTask* task, PipelineTaskState
state, Status exec_status
task->fragment_context()->close_a_pipeline();
}
-void TaskScheduler::_do_work(size_t index) {
+Status TaskScheduler::_do_work(size_t index) {
Review Comment:
warning: method '_do_work' can be made static
[readability-convert-member-functions-to-static]
be/src/pipeline/task_scheduler.h:104:
```diff
- Status _do_work(size_t index);
+ static Status _do_work(size_t index);
```
##########
be/src/pipeline/task_scheduler.cpp:
##########
@@ -248,7 +249,7 @@
task->fragment_context()->close_a_pipeline();
}
-void TaskScheduler::_do_work(size_t index) {
+Status TaskScheduler::_do_work(size_t index) {
Review Comment:
warning: function '_do_work' exceeds recommended size/complexity thresholds
[readability-function-size]
```cpp
Status TaskScheduler::_do_work(size_t index) {
^
```
<details>
<summary>Additional context</summary>
**be/src/pipeline/task_scheduler.cpp:251:** 166 lines including whitespace
and comments (threshold 80)
```cpp
Status TaskScheduler::_do_work(size_t index) {
^
```
</details>
##########
be/src/olap/rowset/beta_rowset_writer.cpp:
##########
@@ -605,10 +605,10 @@ int64_t BetaRowsetWriter::_num_seg() const {
// Eg. rowset schema: A(int), B(float), C(int), D(int)
// _tabelt->tablet_schema: A(bigint), B(double)
// => update_schema: A(bigint), B(double), C(int), D(int)
-void BaseBetaRowsetWriter::update_rowset_schema(TabletSchemaSPtr flush_schema)
{
+Status BaseBetaRowsetWriter::update_rowset_schema(TabletSchemaSPtr
flush_schema) {
Review Comment:
warning: method 'update_rowset_schema' can be made static
[readability-convert-member-functions-to-static]
be/src/olap/rowset/beta_rowset_writer.h:129:
```diff
- Status update_rowset_schema(TabletSchemaSPtr flush_schema);
+ static Status update_rowset_schema(TabletSchemaSPtr flush_schema);
```
##########
be/src/olap/rowset/segment_v2/column_reader.cpp:
##########
@@ -1054,10 +1056,10 @@
return seek_to_ordinal(_page.first_ordinal);
}
-void FileColumnIterator::_seek_to_pos_in_page(ParsedPage* page, ordinal_t
offset_in_page) const {
+Status FileColumnIterator::_seek_to_pos_in_page(ParsedPage* page, ordinal_t
offset_in_page) const {
Review Comment:
warning: method '_seek_to_pos_in_page' can be made static
[readability-convert-member-functions-to-static]
```suggestion
Status FileColumnIterator::_seek_to_pos_in_page(ParsedPage* page, ordinal_t
offset_in_page) {
```
be/src/olap/rowset/segment_v2/column_reader.h:365:
```diff
- Status _seek_to_pos_in_page(ParsedPage* page, ordinal_t
offset_in_page) const;
+ static Status _seek_to_pos_in_page(ParsedPage* page, ordinal_t
offset_in_page) ;
```
--
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
