This is an automated email from the ASF dual-hosted git repository. morningman pushed a commit to branch branch-1.2-lts in repository https://gitbox.apache.org/repos/asf/doris.git
commit 54e9cc7fe307034e9fc2a6b5f059015c15b0464e Author: TengJianPing <18241664+jackte...@users.noreply.github.com> AuthorDate: Tue Dec 6 10:31:15 2022 +0800 [fix](const_expr) fix coredump caused by unsupported cast const expr (#14825) --- be/src/vec/exec/scan/new_olap_scan_node.cpp | 26 ++++--- be/src/vec/exec/scan/new_olap_scan_node.h | 7 +- be/src/vec/exec/scan/vscan_node.cpp | 91 ++++++++++++++-------- be/src/vec/exec/scan/vscan_node.h | 21 ++--- be/src/vec/exprs/varray_literal.cpp | 4 +- be/src/vec/exprs/vexpr.cpp | 19 +++-- be/src/vec/exprs/vexpr.h | 2 +- be/src/vec/exprs/vruntimefilter_wrapper.h | 4 - .../cast_function/test_cast_function.groovy | 17 ++++ 9 files changed, 122 insertions(+), 69 deletions(-) diff --git a/be/src/vec/exec/scan/new_olap_scan_node.cpp b/be/src/vec/exec/scan/new_olap_scan_node.cpp index 64e4c9102a..0f5ba461ed 100644 --- a/be/src/vec/exec/scan/new_olap_scan_node.cpp +++ b/be/src/vec/exec/scan/new_olap_scan_node.cpp @@ -252,12 +252,15 @@ Status NewOlapScanNode::_build_key_ranges_and_filters() { return Status::OK(); } -VScanNode::PushDownType NewOlapScanNode::_should_push_down_function_filter( - VectorizedFnCall* fn_call, VExprContext* expr_ctx, StringVal* constant_str, - doris_udf::FunctionContext** fn_ctx) { +Status NewOlapScanNode::_should_push_down_function_filter(VectorizedFnCall* fn_call, + VExprContext* expr_ctx, + StringVal* constant_str, + doris_udf::FunctionContext** fn_ctx, + VScanNode::PushDownType& pdt) { // Now only `like` function filters is supported to push down if (fn_call->fn().name.function_name != "like") { - return PushDownType::UNACCEPTABLE; + pdt = PushDownType::UNACCEPTABLE; + return Status::OK(); } const auto& children = fn_call->children(); @@ -271,19 +274,24 @@ VScanNode::PushDownType NewOlapScanNode::_should_push_down_function_filter( } if (!children[1 - i]->is_constant()) { // only handle constant value - return PushDownType::UNACCEPTABLE; + pdt = PushDownType::UNACCEPTABLE; + return Status::OK(); } else { DCHECK(children[1 - i]->type().is_string_type()); - if (const ColumnConst* const_column = check_and_get_column<ColumnConst>( - children[1 - i]->get_const_col(expr_ctx)->column_ptr)) { + ColumnPtrWrapper* const_col_wrapper = nullptr; + RETURN_IF_ERROR(children[1 - i]->get_const_col(expr_ctx, &const_col_wrapper)); + if (const ColumnConst* const_column = + check_and_get_column<ColumnConst>(const_col_wrapper->column_ptr)) { *constant_str = const_column->get_data_at(0).to_string_val(); } else { - return PushDownType::UNACCEPTABLE; + pdt = PushDownType::UNACCEPTABLE; + return Status::OK(); } } } *fn_ctx = func_cxt; - return PushDownType::ACCEPTABLE; + pdt = PushDownType::ACCEPTABLE; + return Status::OK(); } // PlanFragmentExecutor will call this method to set scan range diff --git a/be/src/vec/exec/scan/new_olap_scan_node.h b/be/src/vec/exec/scan/new_olap_scan_node.h index 6338c4aee6..4305faf0a8 100644 --- a/be/src/vec/exec/scan/new_olap_scan_node.h +++ b/be/src/vec/exec/scan/new_olap_scan_node.h @@ -39,9 +39,10 @@ protected: Status _process_conjuncts() override; bool _is_key_column(const std::string& col_name) override; - PushDownType _should_push_down_function_filter(VectorizedFnCall* fn_call, - VExprContext* expr_ctx, StringVal* constant_str, - doris_udf::FunctionContext** fn_ctx) override; + Status _should_push_down_function_filter(VectorizedFnCall* fn_call, VExprContext* expr_ctx, + StringVal* constant_str, + doris_udf::FunctionContext** fn_ctx, + PushDownType& pdt) override; PushDownType _should_push_down_bloom_filter() override { return PushDownType::ACCEPTABLE; } diff --git a/be/src/vec/exec/scan/vscan_node.cpp b/be/src/vec/exec/scan/vscan_node.cpp index 870e81d1ed..67dd96f514 100644 --- a/be/src/vec/exec/scan/vscan_node.cpp +++ b/be/src/vec/exec/scan/vscan_node.cpp @@ -356,7 +356,8 @@ Status VScanNode::_normalize_conjuncts() { } if (_vconjunct_ctx_ptr) { if ((*_vconjunct_ctx_ptr)->root()) { - VExpr* new_root = _normalize_predicate((*_vconjunct_ctx_ptr)->root()); + VExpr* new_root; + RETURN_IF_ERROR(_normalize_predicate((*_vconjunct_ctx_ptr)->root(), &new_root)); if (new_root) { (*_vconjunct_ctx_ptr)->set_root(new_root); } else { @@ -380,7 +381,7 @@ Status VScanNode::_normalize_conjuncts() { return Status::OK(); } -VExpr* VScanNode::_normalize_predicate(VExpr* conjunct_expr_root) { +Status VScanNode::_normalize_predicate(VExpr* conjunct_expr_root, VExpr** output_expr) { static constexpr auto is_leaf = [](VExpr* expr) { return !expr->is_and_expr(); }; auto in_predicate_checker = [](const std::vector<VExpr*>& children, const VSlotRef** slot, VExpr** child_contains_slot) { @@ -415,9 +416,10 @@ VExpr* VScanNode::_normalize_predicate(VExpr* conjunct_expr_root) { SlotDescriptor* slot = nullptr; ColumnValueRangeType* range = nullptr; PushDownType pdt = PushDownType::UNACCEPTABLE; - _eval_const_conjuncts(cur_expr, *_vconjunct_ctx_ptr, &pdt); + RETURN_IF_ERROR(_eval_const_conjuncts(cur_expr, *_vconjunct_ctx_ptr, &pdt)); if (pdt == PushDownType::ACCEPTABLE) { - return nullptr; + *output_expr = nullptr; + return Status::OK(); } if (_is_predicate_acting_on_slot(cur_expr, in_predicate_checker, &slot, &range) || _is_predicate_acting_on_slot(cur_expr, eq_predicate_checker, &slot, &range)) { @@ -449,18 +451,23 @@ VExpr* VScanNode::_normalize_predicate(VExpr* conjunct_expr_root) { *range); } if (pdt == PushDownType::ACCEPTABLE && _is_key_column(slot->col_name())) { - return nullptr; + *output_expr = nullptr; + return Status::OK(); } else { // for PARTIAL_ACCEPTABLE and UNACCEPTABLE, do not remove expr from the tree - return conjunct_expr_root; + *output_expr = conjunct_expr_root; + return Status::OK(); } } else { - VExpr* left_child = _normalize_predicate(conjunct_expr_root->children()[0]); - VExpr* right_child = _normalize_predicate(conjunct_expr_root->children()[1]); + VExpr* left_child; + RETURN_IF_ERROR(_normalize_predicate(conjunct_expr_root->children()[0], &left_child)); + VExpr* right_child; + RETURN_IF_ERROR(_normalize_predicate(conjunct_expr_root->children()[1], &right_child)); if (left_child != nullptr && right_child != nullptr) { conjunct_expr_root->set_children({left_child, right_child}); - return conjunct_expr_root; + *output_expr = conjunct_expr_root; + return Status::OK(); } else { // here only close the and expr self, do not close the child conjunct_expr_root->set_children({}); @@ -469,10 +476,12 @@ VExpr* VScanNode::_normalize_predicate(VExpr* conjunct_expr_root) { } // here do not close Expr* now - return left_child != nullptr ? left_child : right_child; + *output_expr = left_child != nullptr ? left_child : right_child; + return Status::OK(); } } - return conjunct_expr_root; + *output_expr = conjunct_expr_root; + return Status::OK(); } Status VScanNode::_normalize_bloom_filter(VExpr* expr, VExprContext* expr_ctx, SlotDescriptor* slot, @@ -516,8 +525,9 @@ Status VScanNode::_normalize_function_filters(VExpr* expr, VExprContext* expr_ct if (TExprNodeType::FUNCTION_CALL == fn_expr->node_type()) { doris_udf::FunctionContext* fn_ctx = nullptr; StringVal val; - PushDownType temp_pdt = _should_push_down_function_filter( - reinterpret_cast<VectorizedFnCall*>(fn_expr), expr_ctx, &val, &fn_ctx); + PushDownType temp_pdt; + RETURN_IF_ERROR(_should_push_down_function_filter( + reinterpret_cast<VectorizedFnCall*>(fn_expr), expr_ctx, &val, &fn_ctx, temp_pdt)); if (temp_pdt != PushDownType::UNACCEPTABLE) { std::string col = slot->col_name(); _push_down_functions.emplace_back(opposite, col, fn_ctx, val); @@ -556,11 +566,13 @@ bool VScanNode::_is_predicate_acting_on_slot( return true; } -void VScanNode::_eval_const_conjuncts(VExpr* vexpr, VExprContext* expr_ctx, PushDownType* pdt) { +Status VScanNode::_eval_const_conjuncts(VExpr* vexpr, VExprContext* expr_ctx, PushDownType* pdt) { char* constant_val = nullptr; if (vexpr->is_constant()) { + ColumnPtrWrapper* const_col_wrapper = nullptr; + RETURN_IF_ERROR(vexpr->get_const_col(expr_ctx, &const_col_wrapper)); if (const ColumnConst* const_column = - check_and_get_column<ColumnConst>(vexpr->get_const_col(expr_ctx)->column_ptr)) { + check_and_get_column<ColumnConst>(const_col_wrapper->column_ptr)) { constant_val = const_cast<char*>(const_column->get_data_at(0).data); if (constant_val == nullptr || *reinterpret_cast<bool*>(constant_val) == false) { *pdt = PushDownType::ACCEPTABLE; @@ -568,14 +580,14 @@ void VScanNode::_eval_const_conjuncts(VExpr* vexpr, VExprContext* expr_ctx, Push } } else if (const ColumnVector<UInt8>* bool_column = check_and_get_column<ColumnVector<UInt8>>( - vexpr->get_const_col(expr_ctx)->column_ptr)) { + const_col_wrapper->column_ptr)) { // TODO: If `vexpr->is_constant()` is true, a const column is expected here. // But now we still don't cover all predicates for const expression. // For example, for query `SELECT col FROM tbl WHERE 'PROMOTION' LIKE 'AAA%'`, // predicate `like` will return a ColumnVector<UInt8> which contains a single value. LOG(WARNING) << "Expr[" << vexpr->debug_string() << "] should return a const column but actually is " - << vexpr->get_const_col(expr_ctx)->column_ptr->get_name(); + << const_col_wrapper->column_ptr->get_name(); DCHECK_EQ(bool_column->size(), 1); if (bool_column->size() == 1) { constant_val = const_cast<char*>(bool_column->get_data_at(0).data); @@ -591,9 +603,10 @@ void VScanNode::_eval_const_conjuncts(VExpr* vexpr, VExprContext* expr_ctx, Push } else { LOG(WARNING) << "Expr[" << vexpr->debug_string() << "] should return a const column but actually is " - << vexpr->get_const_col(expr_ctx)->column_ptr->get_name(); + << const_col_wrapper->column_ptr->get_name(); } } + return Status::OK(); } template <PrimitiveType T> @@ -652,9 +665,10 @@ Status VScanNode::_normalize_in_and_eq_predicate(VExpr* expr, VExprContext* expr StringRef value; int slot_ref_child = -1; - PushDownType temp_pdt = - _should_push_down_binary_predicate(reinterpret_cast<VectorizedFnCall*>(expr), - expr_ctx, &value, &slot_ref_child, eq_checker); + PushDownType temp_pdt; + RETURN_IF_ERROR(_should_push_down_binary_predicate( + reinterpret_cast<VectorizedFnCall*>(expr), expr_ctx, &value, &slot_ref_child, + eq_checker, temp_pdt)); if (temp_pdt == PushDownType::UNACCEPTABLE) { return Status::OK(); } @@ -724,9 +738,10 @@ Status VScanNode::_normalize_not_in_and_not_eq_predicate(VExpr* expr, VExprConte auto ne_checker = [](const std::string& fn_name) { return fn_name == "ne"; }; StringRef value; int slot_ref_child = -1; - if ((temp_pdt = _should_push_down_binary_predicate( - reinterpret_cast<VectorizedFnCall*>(expr), expr_ctx, &value, &slot_ref_child, - ne_checker)) == PushDownType::UNACCEPTABLE) { + RETURN_IF_ERROR(_should_push_down_binary_predicate( + reinterpret_cast<VectorizedFnCall*>(expr), expr_ctx, &value, &slot_ref_child, + ne_checker, temp_pdt)); + if (temp_pdt == PushDownType::UNACCEPTABLE) { return Status::OK(); } @@ -812,9 +827,10 @@ Status VScanNode::_normalize_noneq_binary_predicate(VExpr* expr, VExprContext* e }; StringRef value; int slot_ref_child = -1; - PushDownType temp_pdt = _should_push_down_binary_predicate( + PushDownType temp_pdt; + RETURN_IF_ERROR(_should_push_down_binary_predicate( reinterpret_cast<VectorizedFnCall*>(expr), expr_ctx, &value, &slot_ref_child, - noneq_checker); + noneq_checker, temp_pdt)); if (temp_pdt != PushDownType::UNACCEPTABLE) { DCHECK(slot_ref_child >= 0); const std::string& fn_name = @@ -944,11 +960,13 @@ Status VScanNode::clone_vconjunct_ctx(VExprContext** _vconjunct_ctx) { return Status::OK(); } -VScanNode::PushDownType VScanNode::_should_push_down_binary_predicate( +Status VScanNode::_should_push_down_binary_predicate( VectorizedFnCall* fn_call, VExprContext* expr_ctx, StringRef* constant_val, - int* slot_ref_child, const std::function<bool(const std::string&)>& fn_checker) { + int* slot_ref_child, const std::function<bool(const std::string&)>& fn_checker, + VScanNode::PushDownType& pdt) { if (!fn_checker(fn_call->fn().name.function_name)) { - return PushDownType::UNACCEPTABLE; + pdt = PushDownType::UNACCEPTABLE; + return Status::OK(); } const auto& children = fn_call->children(); @@ -960,18 +978,23 @@ VScanNode::PushDownType VScanNode::_should_push_down_binary_predicate( } if (!children[1 - i]->is_constant()) { // only handle constant value - return PushDownType::UNACCEPTABLE; + pdt = PushDownType::UNACCEPTABLE; + return Status::OK(); } else { - if (const ColumnConst* const_column = check_and_get_column<ColumnConst>( - children[1 - i]->get_const_col(expr_ctx)->column_ptr)) { + ColumnPtrWrapper* const_col_wrapper = nullptr; + RETURN_IF_ERROR(children[1 - i]->get_const_col(expr_ctx, &const_col_wrapper)); + if (const ColumnConst* const_column = + check_and_get_column<ColumnConst>(const_col_wrapper->column_ptr)) { *slot_ref_child = i; *constant_val = const_column->get_data_at(0); } else { - return PushDownType::UNACCEPTABLE; + pdt = PushDownType::UNACCEPTABLE; + return Status::OK(); } } } - return PushDownType::ACCEPTABLE; + pdt = PushDownType::ACCEPTABLE; + return Status::OK(); } VScanNode::PushDownType VScanNode::_should_push_down_in_predicate(VInPredicate* pred, diff --git a/be/src/vec/exec/scan/vscan_node.h b/be/src/vec/exec/scan/vscan_node.h index fbe800ccd6..50bafe309f 100644 --- a/be/src/vec/exec/scan/vscan_node.h +++ b/be/src/vec/exec/scan/vscan_node.h @@ -127,18 +127,21 @@ protected: // 2. in/not in predicate // 3. function predicate // TODO: these interfaces should be change to become more common. - virtual PushDownType _should_push_down_binary_predicate( + virtual Status _should_push_down_binary_predicate( VectorizedFnCall* fn_call, VExprContext* expr_ctx, StringRef* constant_val, - int* slot_ref_child, const std::function<bool(const std::string&)>& fn_checker); + int* slot_ref_child, const std::function<bool(const std::string&)>& fn_checker, + PushDownType& pdt); virtual PushDownType _should_push_down_in_predicate(VInPredicate* in_pred, VExprContext* expr_ctx, bool is_not_in); - virtual PushDownType _should_push_down_function_filter(VectorizedFnCall* fn_call, - VExprContext* expr_ctx, - StringVal* constant_str, - doris_udf::FunctionContext** fn_ctx) { - return PushDownType::UNACCEPTABLE; + virtual Status _should_push_down_function_filter(VectorizedFnCall* fn_call, + VExprContext* expr_ctx, + StringVal* constant_str, + doris_udf::FunctionContext** fn_ctx, + PushDownType& pdt) { + pdt = PushDownType::UNACCEPTABLE; + return Status::OK(); } virtual PushDownType _should_push_down_bloom_filter() { return PushDownType::UNACCEPTABLE; } @@ -262,8 +265,8 @@ private: Status _append_rf_into_conjuncts(std::vector<VExpr*>& vexprs); Status _normalize_conjuncts(); - VExpr* _normalize_predicate(VExpr* conjunct_expr_root); - void _eval_const_conjuncts(VExpr* vexpr, VExprContext* expr_ctx, PushDownType* pdt); + Status _normalize_predicate(VExpr* conjunct_expr_root, VExpr** output_expr); + Status _eval_const_conjuncts(VExpr* vexpr, VExprContext* expr_ctx, PushDownType* pdt); Status _normalize_bloom_filter(VExpr* expr, VExprContext* expr_ctx, SlotDescriptor* slot, PushDownType* pdt); diff --git a/be/src/vec/exprs/varray_literal.cpp b/be/src/vec/exprs/varray_literal.cpp index f2a2c7fa09..1989441b5b 100644 --- a/be/src/vec/exprs/varray_literal.cpp +++ b/be/src/vec/exprs/varray_literal.cpp @@ -28,7 +28,9 @@ Status VArrayLiteral::prepare(RuntimeState* state, const RowDescriptor& row_desc Field array = is_null ? Field() : Array(); for (const auto child : _children) { Field item; - child->get_const_col(context)->column_ptr->get(0, item); + ColumnPtrWrapper* const_col_wrapper = nullptr; + RETURN_IF_ERROR(child->get_const_col(context, &const_col_wrapper)); + const_col_wrapper->column_ptr->get(0, item); array.get<Array>().push_back(item); } _column_ptr = _data_type->create_column_const(1, array); diff --git a/be/src/vec/exprs/vexpr.cpp b/be/src/vec/exprs/vexpr.cpp index 551b128b30..b1dbe2015f 100644 --- a/be/src/vec/exprs/vexpr.cpp +++ b/be/src/vec/exprs/vexpr.cpp @@ -308,13 +308,15 @@ bool VExpr::is_constant() const { return true; } -ColumnPtrWrapper* VExpr::get_const_col(VExprContext* context) { +Status VExpr::get_const_col(VExprContext* context, ColumnPtrWrapper** output) { + *output = nullptr; if (!is_constant()) { - return nullptr; + return Status::OK(); } if (_constant_col != nullptr) { - return _constant_col.get(); + *output = _constant_col.get(); + return Status::OK(); } int result = -1; @@ -322,13 +324,12 @@ ColumnPtrWrapper* VExpr::get_const_col(VExprContext* context) { // If block is empty, some functions will produce no result. So we insert a column with // single value here. block.insert({ColumnUInt8::create(1), std::make_shared<DataTypeUInt8>(), ""}); - if (!execute(context, &block, &result).ok()) { - return nullptr; - } + RETURN_IF_ERROR(execute(context, &block, &result)); DCHECK(result != -1); const auto& column = block.get_by_position(result).column; _constant_col = std::make_shared<ColumnPtrWrapper>(column); - return _constant_col.get(); + *output = _constant_col.get(); + return Status::OK(); } void VExpr::register_function_context(doris::RuntimeState* state, VExprContext* context) { @@ -348,7 +349,9 @@ Status VExpr::init_function_context(VExprContext* context, if (scope == FunctionContext::FRAGMENT_LOCAL) { std::vector<ColumnPtrWrapper*> constant_cols; for (auto c : _children) { - constant_cols.push_back(c->get_const_col(context)); + ColumnPtrWrapper* const_col_wrapper = nullptr; + RETURN_IF_ERROR(c->get_const_col(context, &const_col_wrapper)); + constant_cols.push_back(const_col_wrapper); } fn_ctx->impl()->set_constant_cols(constant_cols); } diff --git a/be/src/vec/exprs/vexpr.h b/be/src/vec/exprs/vexpr.h index 3b81e7bbc8..b1f37477c4 100644 --- a/be/src/vec/exprs/vexpr.h +++ b/be/src/vec/exprs/vexpr.h @@ -148,7 +148,7 @@ public: /// the output. Returns nullptr if the argument is not constant. The returned ColumnPtr is /// owned by this expr. This should only be called after Open() has been called on this /// expr. - virtual ColumnPtrWrapper* get_const_col(VExprContext* context); + Status get_const_col(VExprContext* context, ColumnPtrWrapper** output); int fn_context_index() const { return _fn_context_index; }; diff --git a/be/src/vec/exprs/vruntimefilter_wrapper.h b/be/src/vec/exprs/vruntimefilter_wrapper.h index 624e4918d8..b98e7b9718 100644 --- a/be/src/vec/exprs/vruntimefilter_wrapper.h +++ b/be/src/vec/exprs/vruntimefilter_wrapper.h @@ -40,10 +40,6 @@ public: } const std::string& expr_name() const override; - ColumnPtrWrapper* get_const_col(VExprContext* context) override { - return _impl->get_const_col(context); - } - const VExpr* get_impl() const override { return _impl; } // if filter rate less than this, bloom filter will set always true diff --git a/regression-test/suites/query_p0/sql_functions/cast_function/test_cast_function.groovy b/regression-test/suites/query_p0/sql_functions/cast_function/test_cast_function.groovy index 4dd916141a..799cd9197d 100644 --- a/regression-test/suites/query_p0/sql_functions/cast_function/test_cast_function.groovy +++ b/regression-test/suites/query_p0/sql_functions/cast_function/test_cast_function.groovy @@ -25,6 +25,23 @@ suite("test_cast_function") { qt_sql """ select cast (NULL AS CHAR(1)); """ qt_sql """ select cast ('20190101' AS CHAR(2)); """ + test { + sql """ + select + ref_0.`k0` as c1 + from + `test_query_db`.`baseall` as ref_0 + where + cast( + case + when BITMAP_EMPTY() is NULL then null + else null + end as bitmap + ) is NULL + """ + exception "errCode = 2, detailMessage = Conversion from UInt8 to BitMap is not supported" + } + sql """ SET enable_vectorized_engine = FALSE; """ qt_sql """ select cast (1 as BIGINT) """ --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org