github-actions[bot] commented on code in PR #28493:
URL: https://github.com/apache/doris/pull/28493#discussion_r1427875369


##########
be/src/olap/schema_change.cpp:
##########
@@ -262,8 +262,7 @@ ColumnMapping* 
BlockChanger::get_mutable_column_mapping(size_t column_index) {
 
 Status BlockChanger::change_block(vectorized::Block* ref_block,

Review Comment:
   warning: function 'change_block' has cognitive complexity of 68 (threshold 
50) [readability-function-cognitive-complexity]
   ```cpp
   Status BlockChanger::change_block(vectorized::Block* ref_block,
                        ^
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/olap/schema_change.cpp:270:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       if (_where_expr != nullptr) {
       ^
   ```
   **be/src/olap/schema_change.cpp:272:** +2, including nesting penalty of 1, 
nesting level increased to 2
   ```cpp
           RETURN_IF_ERROR(vectorized::VExpr::create_expr_tree(*_where_expr, 
ctx));
           ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:272:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
           RETURN_IF_ERROR(vectorized::VExpr::create_expr_tree(*_where_expr, 
ctx));
           ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:273:** +2, including nesting penalty of 1, 
nesting level increased to 2
   ```cpp
           RETURN_IF_ERROR(ctx->prepare(state.get(), row_desc));
           ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:273:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
           RETURN_IF_ERROR(ctx->prepare(state.get(), row_desc));
           ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:274:** +2, including nesting penalty of 1, 
nesting level increased to 2
   ```cpp
           RETURN_IF_ERROR(ctx->open(state.get()));
           ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:274:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
           RETURN_IF_ERROR(ctx->open(state.get()));
           ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:276:** +2, including nesting penalty of 1, 
nesting level increased to 2
   ```cpp
           RETURN_IF_ERROR(
           ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:276:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
           RETURN_IF_ERROR(
           ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:285:** +1, including nesting penalty of 0, 
nesting level increased to 1
   ```cpp
       for (int idx = 0; idx < column_size; idx++) {
       ^
   ```
   **be/src/olap/schema_change.cpp:286:** +2, including nesting penalty of 1, 
nesting level increased to 2
   ```cpp
           if (_schema_mapping[idx].expr != nullptr) {
           ^
   ```
   **be/src/olap/schema_change.cpp:288:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
               
RETURN_IF_ERROR(vectorized::VExpr::create_expr_tree(*_schema_mapping[idx].expr, 
ctx));
               ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:288:** +4, including nesting penalty of 3, 
nesting level increased to 4
   ```cpp
               
RETURN_IF_ERROR(vectorized::VExpr::create_expr_tree(*_schema_mapping[idx].expr, 
ctx));
               ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:289:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
               RETURN_IF_ERROR(ctx->prepare(state.get(), row_desc));
               ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:289:** +4, including nesting penalty of 3, 
nesting level increased to 4
   ```cpp
               RETURN_IF_ERROR(ctx->prepare(state.get(), row_desc));
               ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:290:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
               RETURN_IF_ERROR(ctx->open(state.get()));
               ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:290:** +4, including nesting penalty of 3, 
nesting level increased to 4
   ```cpp
               RETURN_IF_ERROR(ctx->open(state.get()));
               ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:293:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
               RETURN_IF_ERROR(ctx->execute(ref_block, &result_column_id));
               ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:293:** +4, including nesting penalty of 3, 
nesting level increased to 4
   ```cpp
               RETURN_IF_ERROR(ctx->execute(ref_block, &result_column_id));
               ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:296:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
               if (ref_block->get_by_position(result_column_id).column->size() 
!= row_size) {
               ^
   ```
   **be/src/olap/schema_change.cpp:301:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
               
RETURN_IF_ERROR(_check_cast_valid(ref_block->get_by_position(idx).column,
               ^
   ```
   **be/src/common/status.h:532:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
       do {                                \
       ^
   ```
   **be/src/olap/schema_change.cpp:301:** +4, including nesting penalty of 3, 
nesting level increased to 4
   ```cpp
               
RETURN_IF_ERROR(_check_cast_valid(ref_block->get_by_position(idx).column,
               ^
   ```
   **be/src/common/status.h:534:** expanded from macro 'RETURN_IF_ERROR'
   ```cpp
           if (UNLIKELY(!_status_.ok())) { \
           ^
   ```
   **be/src/olap/schema_change.cpp:305:** +1, nesting level increased to 2
   ```cpp
           } else if (_schema_mapping[idx].ref_column < 0) {
                  ^
   ```
   **be/src/olap/schema_change.cpp:309:** +3, including nesting penalty of 2, 
nesting level increased to 3
   ```cpp
               if (value->is_null()) {
               ^
   ```
   **be/src/olap/schema_change.cpp:312:** +1, nesting level increased to 3
   ```cpp
               } else {
                 ^
   ```
   **be/src/olap/schema_change.cpp:317:** +1, nesting level increased to 2
   ```cpp
           } else {
             ^
   ```
   
   </details>
   



##########
be/src/vec/exec/scan/scanner_scheduler.cpp:
##########
@@ -193,7 +193,13 @@ void ScannerScheduler::_schedule_thread(int queue_id) {
     return nullptr;
 }
 
-void ScannerScheduler::_schedule_scanners(ScannerContext* ctx) {
+void ScannerScheduler::_schedule_scanners(std::shared_ptr<ScannerContext> ctx) 
{

Review Comment:
   warning: function '_schedule_scanners' exceeds recommended size/complexity 
thresholds [readability-function-size]
   ```cpp
   void ScannerScheduler::_schedule_scanners(std::shared_ptr<ScannerContext> 
ctx) {
                          ^
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/vec/exec/scan/scanner_scheduler.cpp:195:** 102 lines including 
whitespace and comments (threshold 80)
   ```cpp
   void ScannerScheduler::_schedule_scanners(std::shared_ptr<ScannerContext> 
ctx) {
                          ^
   ```
   
   </details>
   



##########
be/src/vec/exec/scan/scanner_scheduler.cpp:
##########
@@ -291,8 +297,14 @@
     ctx->incr_ctx_scheduling_time(watch.elapsed_time());
 }
 
-void ScannerScheduler::_scanner_scan(ScannerScheduler* scheduler, 
ScannerContext* ctx,
-                                     VScannerSPtr scanner) {
+void ScannerScheduler::_scanner_scan(ScannerScheduler* scheduler,

Review Comment:
   warning: method '_scanner_scan' can be made static 
[readability-convert-member-functions-to-static]
   
   be/src/vec/exec/scan/scanner_scheduler.h:86:
   ```diff
   -     void _scanner_scan(ScannerScheduler* scheduler, 
std::shared_ptr<ScannerContext> ctx,
   +     static void _scanner_scan(ScannerScheduler* scheduler, 
std::shared_ptr<ScannerContext> ctx,
   ```
   



##########
be/src/vec/exec/scan/scanner_context.h:
##########
@@ -65,14 +65,13 @@ class SimplifiedScanScheduler;
 // ScannerContext is also the scheduling unit of ScannerScheduler.
 // ScannerScheduler schedules a ScannerContext at a time,
 // and submits the Scanners to the scanner thread pool for data scanning.
-class ScannerContext {
+class ScannerContext : public std::enable_shared_from_this<ScannerContext> {
     ENABLE_FACTORY_CREATOR(ScannerContext);
 
 public:
-    ScannerContext(RuntimeState* state_, VScanNode* parent,
-                   const TupleDescriptor* output_tuple_desc,
-                   const std::list<VScannerSPtr>& scanners_, int64_t limit_,
-                   int64_t max_bytes_in_blocks_queue_, const int 
num_parallel_instances = 1,
+    ScannerContext(RuntimeState* state, VScanNode* parent, const 
TupleDescriptor* output_tuple_desc,
+                   const std::list<VScannerSPtr>& scanners, int64_t limit_,
+                   int64_t max_bytes_in_blocks_queue, const int 
num_parallel_instances = 1,

Review Comment:
   warning: parameter 'num_parallel_instances' 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
                      int64_t max_bytes_in_blocks_queue, int 
num_parallel_instances = 1,
   ```
   



##########
be/src/vec/exec/scan/scanner_scheduler.cpp:
##########
@@ -291,8 +297,14 @@
     ctx->incr_ctx_scheduling_time(watch.elapsed_time());
 }
 
-void ScannerScheduler::_scanner_scan(ScannerScheduler* scheduler, 
ScannerContext* ctx,
-                                     VScannerSPtr scanner) {
+void ScannerScheduler::_scanner_scan(ScannerScheduler* scheduler,

Review Comment:
   warning: function '_scanner_scan' exceeds recommended size/complexity 
thresholds [readability-function-size]
   ```cpp
   void ScannerScheduler::_scanner_scan(ScannerScheduler* scheduler,
                          ^
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/vec/exec/scan/scanner_scheduler.cpp:299:** 138 lines including 
whitespace and comments (threshold 80)
   ```cpp
   void ScannerScheduler::_scanner_scan(ScannerScheduler* scheduler,
                          ^
   ```
   
   </details>
   



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