github-actions[bot] commented on code in PR #64010:
URL: https://github.com/apache/doris/pull/64010#discussion_r3345545435
##########
be/src/storage/segment/segment_iterator.cpp:
##########
@@ -3588,32 +3498,18 @@ Status
SegmentIterator::_materialization_of_virtual_column(Block* block) {
for (const auto& cid_and_expr : _virtual_column_exprs) {
auto cid = cid_and_expr.first;
auto column_expr = cid_and_expr.second;
- size_t idx_in_block = _vir_cid_to_idx_in_block[cid];
- if (block->columns() <= idx_in_block) {
- return Status::InternalError(
- "Virtual column index {} is out of range, block columns
{}, "
- "virtual columns size {}, virtual column expr {}",
- idx_in_block, block->columns(),
_vir_cid_to_idx_in_block.size(),
- column_expr->root()->debug_string());
- } else if (block->get_by_position(idx_in_block).column.get() ==
nullptr) {
- return Status::InternalError(
- "Virtual column index {} is null, block columns {},
virtual columns size {}, "
- "virtual column expr {}",
- idx_in_block, block->columns(),
_vir_cid_to_idx_in_block.size(),
- column_expr->root()->debug_string());
- }
- if (check_and_get_column<const ColumnNothing>(
- block->get_by_position(idx_in_block).column.get())) {
+ auto idx_in_block = _schema_block_id_map[cid];
+ auto& column = block->get_by_position(idx_in_block).column;
+ if (check_and_get_column<const ColumnNothing>(column.get())) {
VLOG_DEBUG << fmt::format("Virtual column is doing
materialization, cid {}, col idx {}",
cid, idx_in_block);
ColumnPtr result_column;
- RETURN_IF_ERROR(column_expr->execute(block, result_column));
+ Block project_block;
+ RETURN_IF_ERROR(_build_project_block(block, &project_block));
+ RETURN_IF_ERROR(column_expr->root()->execute_column(
+ column_expr.get(), &project_block, nullptr,
_selected_size, result_column));
Review Comment:
This replaces `VExprContext::execute(...)`, which wraps expression
evaluation in `RETURN_IF_CATCH_EXCEPTION`, with a raw `execute_column(...)`
call. Virtual column fallback can evaluate arbitrary expressions, including
functions that throw Doris `Exception` (the code above even calls out
`element_at` as a problematic function on empty input). With this change those
exceptions escape `_materialization_of_virtual_column()` instead of being
converted to `Status`, which violates the storage iterator error boundary and
can abort the scan path instead of returning a query error. Please keep the
projected-block evaluation but wrap the `execute_column` call with
`RETURN_IF_CATCH_EXCEPTION` and then check the resulting `Status`, or add an
equivalent helper on `VExprContext` that accepts the explicit row count.
--
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]
