This is an automated email from the ASF dual-hosted git repository. morningman pushed a commit to branch dev-1.0.1 in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
commit b01bd5b72eb90c8795f0db0c3f2d9a31e20ae244 Author: Pxl <[email protected]> AuthorDate: Wed Jun 1 15:19:31 2022 +0800 [fix](vectorized) fix vcast expr input wrong row number (#9520) --- be/src/vec/exprs/vcast_expr.cpp | 16 +++++------ be/src/vec/exprs/vexpr.h | 11 ++++++++ be/src/vec/exprs/vinfo_func.cpp | 9 ++---- be/src/vec/exprs/vliteral.cpp | 10 ++----- be/src/vec/functions/function_cast.h | 54 ++++++++++-------------------------- 5 files changed, 39 insertions(+), 61 deletions(-) diff --git a/be/src/vec/exprs/vcast_expr.cpp b/be/src/vec/exprs/vcast_expr.cpp index a1a441b7b8..f910dfd466 100644 --- a/be/src/vec/exprs/vcast_expr.cpp +++ b/be/src/vec/exprs/vcast_expr.cpp @@ -19,8 +19,10 @@ #include <string_view> +#include "common/status.h" #include "vec/core/field.h" #include "vec/data_types/data_type_factory.hpp" +#include "vec/exprs/vexpr.h" #include "vec/functions/simple_function_factory.h" namespace doris::vectorized { @@ -72,21 +74,19 @@ void VCastExpr::close(doris::RuntimeState* state, VExprContext* context, doris::Status VCastExpr::execute(VExprContext* context, doris::vectorized::Block* block, int* result_column_id) { // for each child call execute - doris::vectorized::ColumnNumbers arguments(2); - int column_id = -1; + int column_id = 0; _children[0]->execute(context, block, &column_id); - arguments[0] = column_id; - size_t const_param_id = block->columns(); - block->insert({_cast_param, _cast_param_data_type, _target_data_type_name}); - arguments[1] = const_param_id; + size_t const_param_id = VExpr::insert_param( + block, {_cast_param, _cast_param_data_type, _target_data_type_name}, block->rows()); // call function size_t num_columns_without_result = block->columns(); // prepare a column to save result block->insert({nullptr, _data_type, _expr_name}); - _function->execute(context->fn_context(_fn_context_index), *block, arguments, - num_columns_without_result, block->rows(), false); + RETURN_IF_ERROR(_function->execute(context->fn_context(_fn_context_index), *block, + {static_cast<size_t>(column_id), const_param_id}, + num_columns_without_result, block->rows(), false)); *result_column_id = num_columns_without_result; return Status::OK(); } diff --git a/be/src/vec/exprs/vexpr.h b/be/src/vec/exprs/vexpr.h index 0bbce41af8..8958b91e00 100644 --- a/be/src/vec/exprs/vexpr.h +++ b/be/src/vec/exprs/vexpr.h @@ -33,8 +33,19 @@ namespace vectorized { class VExpr { public: + // resize inserted param column to make sure column size equal to block.rows() + // and return param column index + static size_t insert_param(Block* block, ColumnWithTypeAndName&& elem, size_t size) { + // usualy elem.column always is const column, so we just clone it. + elem.column = elem.column->clone_resized(size); + block->insert(std::move(elem)); + return block->columns() - 1; + } + VExpr(const TExprNode& node); VExpr(const TypeDescriptor& type, bool is_slotref, bool is_nullable); + // only used for test + VExpr() = default; virtual ~VExpr() = default; virtual VExpr* clone(ObjectPool* pool) const = 0; diff --git a/be/src/vec/exprs/vinfo_func.cpp b/be/src/vec/exprs/vinfo_func.cpp index d703c3790f..d347d97240 100644 --- a/be/src/vec/exprs/vinfo_func.cpp +++ b/be/src/vec/exprs/vinfo_func.cpp @@ -47,12 +47,9 @@ VInfoFunc::VInfoFunc(const TExprNode& node) : VExpr(node) { } Status VInfoFunc::execute(VExprContext* context, vectorized::Block* block, int* result_column_id) { - int rows = block->rows(); - if (rows < 1) { - rows = 1; - } - *result_column_id = block->columns(); - block->insert({_column_ptr->clone_resized(rows), _data_type, _expr_name}); + // Info function should return least one row, e.g. select current_user(). + size_t row_size = std::max(block->rows(), size_t(1)); + *result_column_id = VExpr::insert_param(block, {_column_ptr, _data_type, _expr_name}, row_size); return Status::OK(); } diff --git a/be/src/vec/exprs/vliteral.cpp b/be/src/vec/exprs/vliteral.cpp index 38ec4e1bf6..37088d0a9f 100644 --- a/be/src/vec/exprs/vliteral.cpp +++ b/be/src/vec/exprs/vliteral.cpp @@ -126,13 +126,9 @@ VLiteral::VLiteral(const TExprNode& node) : VExpr(node) { VLiteral::~VLiteral() {} Status VLiteral::execute(VExprContext* context, vectorized::Block* block, int* result_column_id) { - int rows = block->rows(); - if (rows < 1) { - rows = 1; - } - size_t res = block->columns(); - block->insert({_column_ptr->clone_resized(rows), _data_type, _expr_name}); - *result_column_id = res; + // Literal expr should return least one row. + size_t row_size = std::max(block->rows(), size_t(1)); + *result_column_id = VExpr::insert_param(block, {_column_ptr, _data_type, _expr_name}, row_size); return Status::OK(); } } // namespace doris::vectorized diff --git a/be/src/vec/functions/function_cast.h b/be/src/vec/functions/function_cast.h index 092842f5fa..893b146840 100644 --- a/be/src/vec/functions/function_cast.h +++ b/be/src/vec/functions/function_cast.h @@ -718,11 +718,7 @@ struct ConvertThroughParsing { using ToFieldType = typename ToDataType::FieldType; - static bool is_all_read(ReadBuffer& in) { - if (in.eof()) return true; - - return false; - } + static bool is_all_read(ReadBuffer& in) { return in.eof(); } template <typename Additions = void*> static Status execute(Block& block, const ColumnNumbers& arguments, size_t result, @@ -779,19 +775,9 @@ struct ConvertThroughParsing { ReadBuffer read_buffer(&(*chars)[current_offset], string_size); - { - bool parsed; - - { - parsed = try_parse_impl<ToDataType>(vec_to[i], read_buffer, local_time_zone); - - parsed = parsed && is_all_read(read_buffer); - } - - if (!parsed) vec_to[i] = 0; - - (*vec_null_map_to)[i] = !parsed; - } + (*vec_null_map_to)[i] = + !try_parse_impl<ToDataType>(vec_to[i], read_buffer, local_time_zone) || + !is_all_read(read_buffer); current_offset = next_offset; } @@ -836,27 +822,16 @@ public: size_t result, size_t input_rows_count) override { const IDataType* from_type = block.get_by_position(arguments[0]).type.get(); - bool ok = true; - - { - if (check_and_get_data_type<DataTypeString>(from_type)) { - return ConvertThroughParsing<DataTypeString, ToDataType, Name>::execute( - block, arguments, result, input_rows_count); - } - - else - ok = false; - } - - if (!ok) { - return Status::RuntimeError(fmt::format( - "Illegal type {} of argument of function {} . Only String or FixedString " - "argument is accepted for try-conversion function. For other arguments, use " - "function without 'orZero' or 'orNull'.", - block.get_by_position(arguments[0]).type->get_name(), get_name())); + if (check_and_get_data_type<DataTypeString>(from_type)) { + return ConvertThroughParsing<DataTypeString, ToDataType, Name>::execute( + block, arguments, result, input_rows_count); } - return Status::OK(); + return Status::RuntimeError(fmt::format( + "Illegal type {} of argument of function {} . Only String or FixedString " + "argument is accepted for try-conversion function. For other arguments, use " + "function without 'orZero' or 'orNull'.", + block.get_by_position(arguments[0]).type->get_name(), get_name())); } }; @@ -877,8 +852,7 @@ public: // This function should not be called for get DateType Ptr // using the FunctionCast::get_return_type_impl DataTypePtr get_return_type_impl(const ColumnsWithTypeAndName& arguments) const override { - auto res = std::make_shared<ToDataType>(); - return res; + return std::make_shared<ToDataType>(); } Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments, @@ -1084,7 +1058,7 @@ private: }; } - bool skip_not_null_check = false; + constexpr bool skip_not_null_check = false; auto wrapper = prepare_remove_nullable(from_nested, to_nested, skip_not_null_check); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
