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


##########
be/src/vec/functions/if.cpp:
##########
@@ -321,23 +321,25 @@
                                                       
else_col.type->get_type_id(), call);
     }
 
-    bool execute_for_null_then_else(FunctionContext* context, Block& block,
-                                    const ColumnWithTypeAndName& arg_cond,
-                                    const ColumnWithTypeAndName& arg_then,
-                                    const ColumnWithTypeAndName& arg_else, 
size_t result,
-                                    size_t input_rows_count, Status& status) 
const {
+    Status execute_for_null_then_else(FunctionContext* context, Block& block,

Review Comment:
   warning: function 'execute_for_null_then_else' exceeds recommended 
size/complexity thresholds [readability-function-size]
   ```cpp
       Status execute_for_null_then_else(FunctionContext* context, Block& block,
              ^
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/vec/functions/if.cpp:323:** 95 lines including whitespace and 
comments (threshold 80)
   ```cpp
       Status execute_for_null_then_else(FunctionContext* context, Block& block,
              ^
   ```
   
   </details>
   



##########
be/src/vec/functions/if.cpp:
##########
@@ -321,23 +321,25 @@ class FunctionIf : public IFunction {
                                                       
else_col.type->get_type_id(), call);
     }
 
-    bool execute_for_null_then_else(FunctionContext* context, Block& block,
-                                    const ColumnWithTypeAndName& arg_cond,
-                                    const ColumnWithTypeAndName& arg_then,
-                                    const ColumnWithTypeAndName& arg_else, 
size_t result,
-                                    size_t input_rows_count, Status& status) 
const {
+    Status execute_for_null_then_else(FunctionContext* context, Block& block,

Review Comment:
   warning: method 'execute_for_null_then_else' can be made static 
[readability-convert-member-functions-to-static]
   
   ```suggestion
       static Status execute_for_null_then_else(FunctionContext* context, 
Block& block,
   ```
   
   be/src/vec/functions/if.cpp:327:
   ```diff
   -                                       size_t input_rows_count, bool& 
handled) const {
   +                                       size_t input_rows_count, bool& 
handled) {
   ```
   



##########
be/src/vec/functions/if.cpp:
##########
@@ -498,35 +496,39 @@
                      {get_nested_column(arg_else.column), 
remove_nullable(arg_else.type), ""},
                      {nullptr, 
remove_nullable(block.get_by_position(result).type), ""}});
 
-            static_cast<void>(_execute_impl_internal(context, temporary_block, 
{0, 1, 2}, 3,
-                                                     temporary_block.rows()));
+            RETURN_IF_ERROR(_execute_impl_internal(context, temporary_block, 
{0, 1, 2}, 3,
+                                                   temporary_block.rows()));
 
             result_nested_column = temporary_block.get_by_position(3).column;
         }
 
         auto column = 
ColumnNullable::create(materialize_column_if_const(result_nested_column),
                                              
materialize_column_if_const(result_null_mask));
         block.replace_by_position(result, std::move(column));
-        return true;
+        handled = true;
+        return Status::OK();
     }
 
-    bool execute_for_null_condition(FunctionContext* context, Block& block,
-                                    const ColumnNumbers& arguments,
-                                    const ColumnWithTypeAndName& arg_cond,
-                                    const ColumnWithTypeAndName& arg_then,
-                                    const ColumnWithTypeAndName& arg_else, 
size_t result) const {
+    Status execute_for_null_condition(FunctionContext* context, Block& block,

Review Comment:
   warning: method 'execute_for_null_condition' can be made static 
[readability-convert-member-functions-to-static]
   
   ```suggestion
       static Status execute_for_null_condition(FunctionContext* context, 
Block& block,
   ```
   
   be/src/vec/functions/if.cpp:516:
   ```diff
   -                                       bool& handled) const {
   +                                       bool& handled) {
   ```
   



##########
be/src/vec/functions/if.cpp:
##########
@@ -414,41 +412,41 @@
                                     input_rows_count);
                 }
             } else {
-                status = Status::InternalError(
+                return Status::InternalError(
                         "Illegal column {} of first argument of function {}. 
Must be ColumnUInt8 "
                         "or ColumnConstUInt8.",
                         arg_cond.column->get_name(), get_name());
             }
-            return true;
         }
-
-        return false;
+        handled = true;
+        return Status::OK();
     }
 
-    bool execute_for_nullable_then_else(FunctionContext* context, Block& block,
-                                        const ColumnWithTypeAndName& arg_cond,
-                                        const ColumnWithTypeAndName& arg_then,
-                                        const ColumnWithTypeAndName& arg_else, 
size_t result,
-                                        size_t input_rows_count) const {
+    Status execute_for_nullable_then_else(FunctionContext* context, Block& 
block,

Review Comment:
   warning: function 'execute_for_nullable_then_else' exceeds recommended 
size/complexity thresholds [readability-function-size]
   ```cpp
       Status execute_for_nullable_then_else(FunctionContext* context, Block& 
block,
              ^
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/vec/functions/if.cpp:424:** 81 lines including whitespace and 
comments (threshold 80)
   ```cpp
       Status execute_for_nullable_then_else(FunctionContext* context, Block& 
block,
              ^
   ```
   
   </details>
   



##########
be/src/vec/functions/if.cpp:
##########
@@ -414,41 +412,41 @@
                                     input_rows_count);
                 }
             } else {
-                status = Status::InternalError(
+                return Status::InternalError(
                         "Illegal column {} of first argument of function {}. 
Must be ColumnUInt8 "
                         "or ColumnConstUInt8.",
                         arg_cond.column->get_name(), get_name());
             }
-            return true;
         }
-
-        return false;
+        handled = true;
+        return Status::OK();
     }
 
-    bool execute_for_nullable_then_else(FunctionContext* context, Block& block,
-                                        const ColumnWithTypeAndName& arg_cond,
-                                        const ColumnWithTypeAndName& arg_then,
-                                        const ColumnWithTypeAndName& arg_else, 
size_t result,
-                                        size_t input_rows_count) const {
+    Status execute_for_nullable_then_else(FunctionContext* context, Block& 
block,

Review Comment:
   warning: method 'execute_for_nullable_then_else' can be made static 
[readability-convert-member-functions-to-static]
   
   ```suggestion
       static Status execute_for_nullable_then_else(FunctionContext* context, 
Block& block,
   ```
   
   be/src/vec/functions/if.cpp:428:
   ```diff
   -                                           size_t input_rows_count, bool& 
handled) const {
   +                                           size_t input_rows_count, bool& 
handled) {
   ```
   



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