Copilot commented on code in PR #60389:
URL: https://github.com/apache/doris/pull/60389#discussion_r2745219982


##########
be/src/vec/exprs/vcase_expr.cpp:
##########
@@ -116,7 +116,10 @@ Status VCaseExpr::execute_column(VExprContext* context, 
const Block* block, size
     } else {
         result_column = _execute_impl<uint8_t>(when_columns, then_columns, 
rows_count);
     }
-    DCHECK_EQ(result_column->size(), count);
+    if (result_column->size() != count) {
+        return Status::InternalError("case when result column size {} not 
equal input count {}",
+                                     result_column->size(), count);
+    }

Review Comment:
   This conversion from DCHECK to runtime error check is inconsistent with the 
codebase. Many other similar expression files still use DCHECK_EQ for the same 
pattern (e.g., vbitmap_predicate.cpp:104, vbloom_predicate.cpp:91, 
vcast_expr.cpp:126, vcondition_expr.cpp:484, vectorized_fn_call.cpp:246, and 
many others). For consistency, either this change should be reverted to keep 
using DCHECK, or similar changes should be applied to all the other expression 
execute_column implementations that have the same pattern.



##########
be/src/pipeline/exec/operator.cpp:
##########
@@ -316,16 +316,32 @@ Status OperatorXBase::do_projections(RuntimeState* state, 
vectorized::Block* ori
     size_t bytes_usage = 0;
     vectorized::ColumnsWithTypeAndName new_columns;
     for (const auto& projections : local_state->_intermediate_projections) {
+        if (projections.empty()) {
+            return Status::InternalError("meet empty intermediate projection, 
node id: {}",
+                                         node_id());

Review Comment:
   This check may be overly restrictive. The _intermediate_projections 
container could legitimately be empty in some operator configurations. While 
the check helps catch configuration errors early, it would be more helpful to 
verify whether an empty intermediate projection is a valid configuration for 
certain operators. If not, consider adding more context to the error message 
explaining why empty intermediate projections are invalid.
   ```suggestion
               return Status::InternalError(
                       "meet empty intermediate projection, which is invalid 
for this operator. "
                       "This usually indicates a misconfigured plan or operator 
configuration. "
                       "node id: {}, operator id: {}",
                       node_id(), operator_id());
   ```



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