This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch dev-1.1.2 in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/dev-1.1.2 by this push: new d066af5e2f [enhancement](VSlotRef) enhance column_id check in execute function during runtime (#11862) d066af5e2f is described below commit d066af5e2fc4f0943f574c77fa1157c329dfd42e Author: AlexYue <yj976240...@qq.com> AuthorDate: Thu Aug 18 09:12:26 2022 +0800 [enhancement](VSlotRef) enhance column_id check in execute function during runtime (#11862) The column id check in VSlotRef::execute function before is too strict for fuzzy test to continuously produce random query. Temporarily loosen the check logic. Moreover, there exists some careless call to VExpr::get_const_col, it might return a nullptr but not every function call checks if it's valid. It's an underlying problem. --- .../vec/exprs/table_function/vexplode_bitmap.cpp | 4 +++- .../exprs/table_function/vexplode_json_array.cpp | 3 ++- .../vec/exprs/table_function/vexplode_numbers.cpp | 4 +++- be/src/vec/exprs/table_function/vexplode_split.cpp | 6 +++-- be/src/vec/exprs/vcase_expr.cpp | 3 ++- be/src/vec/exprs/vcast_expr.cpp | 2 +- be/src/vec/exprs/vectorized_fn_call.cpp | 3 ++- be/src/vec/exprs/vin_predicate.cpp | 3 ++- be/src/vec/exprs/vslot_ref.cpp | 26 ++++++++++++---------- 9 files changed, 33 insertions(+), 21 deletions(-) diff --git a/be/src/vec/exprs/table_function/vexplode_bitmap.cpp b/be/src/vec/exprs/table_function/vexplode_bitmap.cpp index 0dec849980..f015ac7330 100644 --- a/be/src/vec/exprs/table_function/vexplode_bitmap.cpp +++ b/be/src/vec/exprs/table_function/vexplode_bitmap.cpp @@ -17,6 +17,7 @@ #include "vec/exprs/table_function/vexplode_bitmap.h" +#include "common/status.h" #include "util/bitmap_value.h" #include "vec/exprs/vexpr.h" @@ -32,7 +33,8 @@ Status VExplodeBitmapTableFunction::process_init(vectorized::Block* block) { << _vexpr_context->root()->children().size(); int value_column_idx = -1; - _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, &value_column_idx); + RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, block, + &value_column_idx)); _value_column = block->get_by_position(value_column_idx).column; return Status::OK(); diff --git a/be/src/vec/exprs/table_function/vexplode_json_array.cpp b/be/src/vec/exprs/table_function/vexplode_json_array.cpp index aedcc53643..8ee1815117 100644 --- a/be/src/vec/exprs/table_function/vexplode_json_array.cpp +++ b/be/src/vec/exprs/table_function/vexplode_json_array.cpp @@ -32,7 +32,8 @@ Status VExplodeJsonArrayTableFunction::process_init(vectorized::Block* block) { << _vexpr_context->root()->children().size(); int text_column_idx = -1; - _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, &text_column_idx); + RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, block, + &text_column_idx)); _text_column = block->get_by_position(text_column_idx).column; return Status::OK(); diff --git a/be/src/vec/exprs/table_function/vexplode_numbers.cpp b/be/src/vec/exprs/table_function/vexplode_numbers.cpp index 540c7ac5df..a76f759630 100644 --- a/be/src/vec/exprs/table_function/vexplode_numbers.cpp +++ b/be/src/vec/exprs/table_function/vexplode_numbers.cpp @@ -17,6 +17,7 @@ #include "vec/exprs/table_function/vexplode_numbers.h" +#include "common/status.h" #include "vec/exprs/vexpr.h" namespace doris::vectorized { @@ -31,7 +32,8 @@ Status VExplodeNumbersTableFunction::process_init(vectorized::Block* block) { << _vexpr_context->root()->children().size(); int value_column_idx = -1; - _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, &value_column_idx); + RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, block, + &value_column_idx)); _value_column = block->get_by_position(value_column_idx).column; return Status::OK(); diff --git a/be/src/vec/exprs/table_function/vexplode_split.cpp b/be/src/vec/exprs/table_function/vexplode_split.cpp index 1e124f5781..fbd37d4d3f 100644 --- a/be/src/vec/exprs/table_function/vexplode_split.cpp +++ b/be/src/vec/exprs/table_function/vexplode_split.cpp @@ -40,8 +40,10 @@ Status VExplodeSplitTableFunction::process_init(vectorized::Block* block) { int text_column_idx = -1; int delimiter_column_idx = -1; - _vexpr_context->root()->children()[0]->execute(_vexpr_context, block, &text_column_idx); - _vexpr_context->root()->children()[1]->execute(_vexpr_context, block, &delimiter_column_idx); + RETURN_IF_ERROR(_vexpr_context->root()->children()[0]->execute(_vexpr_context, block, + &text_column_idx)); + RETURN_IF_ERROR(_vexpr_context->root()->children()[1]->execute(_vexpr_context, block, + &delimiter_column_idx)); _text_column = block->get_by_position(text_column_idx).column; _delimiter_column = block->get_by_position(delimiter_column_idx).column; diff --git a/be/src/vec/exprs/vcase_expr.cpp b/be/src/vec/exprs/vcase_expr.cpp index 2683e33bba..13c98b152c 100644 --- a/be/src/vec/exprs/vcase_expr.cpp +++ b/be/src/vec/exprs/vcase_expr.cpp @@ -17,6 +17,7 @@ #include "vec/exprs/vcase_expr.h" +#include "common/status.h" #include "vec/columns/column_nullable.h" namespace doris::vectorized { @@ -94,7 +95,7 @@ Status VCaseExpr::execute(VExprContext* context, Block* block, int* result_colum for (int i = 0; i < _children.size(); i++) { int column_id = -1; - _children[i]->execute(context, block, &column_id); + RETURN_IF_ERROR(_children[i]->execute(context, block, &column_id)); arguments[i] = column_id; block->replace_by_position_if_const(column_id); diff --git a/be/src/vec/exprs/vcast_expr.cpp b/be/src/vec/exprs/vcast_expr.cpp index f910dfd466..d0286bdce6 100644 --- a/be/src/vec/exprs/vcast_expr.cpp +++ b/be/src/vec/exprs/vcast_expr.cpp @@ -75,7 +75,7 @@ doris::Status VCastExpr::execute(VExprContext* context, doris::vectorized::Block int* result_column_id) { // for each child call execute int column_id = 0; - _children[0]->execute(context, block, &column_id); + RETURN_IF_ERROR(_children[0]->execute(context, block, &column_id)); size_t const_param_id = VExpr::insert_param( block, {_cast_param, _cast_param_data_type, _target_data_type_name}, block->rows()); diff --git a/be/src/vec/exprs/vectorized_fn_call.cpp b/be/src/vec/exprs/vectorized_fn_call.cpp index 6f01a126d6..d0850c9cf2 100644 --- a/be/src/vec/exprs/vectorized_fn_call.cpp +++ b/be/src/vec/exprs/vectorized_fn_call.cpp @@ -19,6 +19,7 @@ #include <string_view> +#include "common/status.h" #include "exprs/anyval_util.h" #include "fmt/format.h" #include "fmt/ranges.h" @@ -79,7 +80,7 @@ doris::Status VectorizedFnCall::execute(VExprContext* context, doris::vectorized doris::vectorized::ColumnNumbers arguments(_children.size()); for (int i = 0; i < _children.size(); ++i) { int column_id = -1; - _children[i]->execute(context, block, &column_id); + RETURN_IF_ERROR(_children[i]->execute(context, block, &column_id)); arguments[i] = column_id; } // call function diff --git a/be/src/vec/exprs/vin_predicate.cpp b/be/src/vec/exprs/vin_predicate.cpp index f8c096d8ee..3a84087bd7 100644 --- a/be/src/vec/exprs/vin_predicate.cpp +++ b/be/src/vec/exprs/vin_predicate.cpp @@ -19,6 +19,7 @@ #include <string_view> +#include "common/status.h" #include "exprs/create_predicate_function.h" #include "vec/columns/column_set.h" @@ -89,7 +90,7 @@ Status VInPredicate::execute(VExprContext* context, Block* block, int* result_co doris::vectorized::ColumnNumbers arguments(_children.size()); for (int i = 0; i < _children.size(); ++i) { int column_id = -1; - _children[i]->execute(context, block, &column_id); + RETURN_IF_ERROR(_children[i]->execute(context, block, &column_id)); arguments[i] = column_id; } // call function diff --git a/be/src/vec/exprs/vslot_ref.cpp b/be/src/vec/exprs/vslot_ref.cpp index 8cbc56cb37..81b845cb97 100644 --- a/be/src/vec/exprs/vslot_ref.cpp +++ b/be/src/vec/exprs/vslot_ref.cpp @@ -26,16 +26,13 @@ namespace doris::vectorized { using doris::Status; using doris::SlotDescriptor; VSlotRef::VSlotRef(const doris::TExprNode& node) - : VExpr(node), - _slot_id(node.slot_ref.slot_id), - _column_id(-1), - _column_name(nullptr) { - if (node.__isset.is_nullable) { - _is_nullable = node.is_nullable; - } else { - _is_nullable = true; - } - } + : VExpr(node), _slot_id(node.slot_ref.slot_id), _column_id(-1), _column_name(nullptr) { + if (node.__isset.is_nullable) { + _is_nullable = node.is_nullable; + } else { + _is_nullable = true; + } +} VSlotRef::VSlotRef(const SlotDescriptor* desc) : VExpr(desc->type(), true, desc->is_nullable()), @@ -60,7 +57,11 @@ Status VSlotRef::prepare(doris::RuntimeState* state, const doris::RowDescriptor& } Status VSlotRef::execute(VExprContext* context, Block* block, int* result_column_id) { - CHECK_GE(_column_id, 0) << ", " << debug_string() << ", " << get_stack_trace(); + // comment DCHECK temporarily to make fuzzy test run smoothly + // DCHECK_GE(_column_id, 0); + if (_column_id < 0) { + return Status::InternalError("invalid column id {}", _column_id); + } *result_column_id = _column_id; return Status::OK(); } @@ -70,7 +71,8 @@ const std::string& VSlotRef::expr_name() const { } std::string VSlotRef::debug_string() const { std::stringstream out; - out << "SlotRef(slot_id=" << _slot_id << VExpr::debug_string() << ") column id: " << _column_id << ", name: " << *_column_name << ", is nulable: " << _is_nullable; + out << "SlotRef(slot_id=" << _slot_id << VExpr::debug_string() << ") column id: " << _column_id + << ", name: " << *_column_name << ", is nulable: " << _is_nullable; return out.str(); } } // namespace doris::vectorized --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org