HappenLee commented on code in PR #18310:
URL: https://github.com/apache/doris/pull/18310#discussion_r1156876241
##########
be/src/vec/functions/function_conv.cpp:
##########
@@ -49,58 +52,99 @@ class FunctionConv : public IFunction {
auto result_column = ColumnString::create();
auto result_null_map_column = ColumnUInt8::create(input_rows_count, 0);
+ bool col_const[3];
ColumnPtr argument_columns[3];
-
for (int i = 0; i < 3; ++i) {
- argument_columns[i] =
-
block.get_by_position(arguments[i]).column->convert_to_full_column_if_const();
- if (auto* nullable =
check_and_get_column<ColumnNullable>(*argument_columns[i])) {
- // Danger: Here must dispose the null map data first! Because
- // argument_columns[i]=nullable->get_nested_column_ptr(); will
release the mem
- // of column nullable mem of null map
-
VectorizedUtils::update_null_map(result_null_map_column->get_data(),
-
nullable->get_null_map_data());
- argument_columns[i] = nullable->get_nested_column_ptr();
- }
+ col_const[i] =
is_column_const(*block.get_by_position(arguments[i]).column);
}
+ argument_columns[0] = col_const[0] ? static_cast<const ColumnConst&>(
+
*block.get_by_position(arguments[0]).column)
+ .convert_to_full_column()
+ :
block.get_by_position(arguments[0]).column;
+
+ default_preprocess_parameter_columns(argument_columns, col_const, {1,
2}, block, arguments);
- execute_straight(
- context,
- assert_cast<const typename
Impl::DataType::ColumnType*>(argument_columns[0].get()),
- assert_cast<const ColumnInt8*>(argument_columns[1].get()),
- assert_cast<const ColumnInt8*>(argument_columns[2].get()),
- assert_cast<ColumnString*>(result_column.get()),
-
assert_cast<ColumnUInt8*>(result_null_map_column.get())->get_data(),
- input_rows_count);
+ for (int i = 0; i < 3; i++) {
+ check_set_nullable(argument_columns[i], result_null_map_column);
+ }
+
+ if (col_const[1] && col_const[2]) {
+ execute_scalar_args(
+ context,
+ assert_cast<const typename Impl::DataType::ColumnType*>(
+ argument_columns[0].get()),
+ assert_cast<const
ColumnInt8*>(argument_columns[1].get())->get_element(0),
+ assert_cast<const
ColumnInt8*>(argument_columns[2].get())->get_element(0),
+ assert_cast<ColumnString*>(result_column.get()),
+
assert_cast<ColumnUInt8*>(result_null_map_column.get())->get_data(),
+ input_rows_count);
+ } else {
+ execute_straight(context,
+ assert_cast<const typename
Impl::DataType::ColumnType*>(
+ argument_columns[0].get()),
+ assert_cast<const
ColumnInt8*>(argument_columns[1].get()),
+ assert_cast<const
ColumnInt8*>(argument_columns[2].get()),
+ assert_cast<ColumnString*>(result_column.get()),
+
assert_cast<ColumnUInt8*>(result_null_map_column.get())->get_data(),
+ input_rows_count);
+ }
block.get_by_position(result).column =
ColumnNullable::create(std::move(result_column),
std::move(result_null_map_column));
return Status::OK();
}
private:
- void execute_straight(FunctionContext* context,
- const typename Impl::DataType::ColumnType*
data_column,
- const ColumnInt8* src_base_column, const ColumnInt8*
dst_base_column,
- ColumnString* result_column, NullMap&
result_null_map,
- size_t input_rows_count) {
+ static bool _check_set_oob(const Int8 src_base, const Int8 dst_base,
+ ColumnString* result_column, UInt8&
result_null) {
+ if (std::abs(src_base) < MathFunctions::MIN_BASE ||
Review Comment:
```
std::abs(src_base) < MathFunctions::MIN_BASE ||
std::abs(src_base) > MathFunctions::MAX_BASE ||
std::abs(dst_base) < MathFunctions::MIN_BASE ||
std::abs(dst_base) > MathFunctions::MAX_BASE
``` should be a function
##########
be/src/vec/functions/function_conv.cpp:
##########
@@ -49,58 +52,99 @@ class FunctionConv : public IFunction {
auto result_column = ColumnString::create();
auto result_null_map_column = ColumnUInt8::create(input_rows_count, 0);
+ bool col_const[3];
ColumnPtr argument_columns[3];
-
for (int i = 0; i < 3; ++i) {
- argument_columns[i] =
-
block.get_by_position(arguments[i]).column->convert_to_full_column_if_const();
- if (auto* nullable =
check_and_get_column<ColumnNullable>(*argument_columns[i])) {
- // Danger: Here must dispose the null map data first! Because
- // argument_columns[i]=nullable->get_nested_column_ptr(); will
release the mem
- // of column nullable mem of null map
-
VectorizedUtils::update_null_map(result_null_map_column->get_data(),
-
nullable->get_null_map_data());
- argument_columns[i] = nullable->get_nested_column_ptr();
- }
+ col_const[i] =
is_column_const(*block.get_by_position(arguments[i]).column);
}
+ argument_columns[0] = col_const[0] ? static_cast<const ColumnConst&>(
+
*block.get_by_position(arguments[0]).column)
+ .convert_to_full_column()
+ :
block.get_by_position(arguments[0]).column;
+
+ default_preprocess_parameter_columns(argument_columns, col_const, {1,
2}, block, arguments);
- execute_straight(
- context,
- assert_cast<const typename
Impl::DataType::ColumnType*>(argument_columns[0].get()),
- assert_cast<const ColumnInt8*>(argument_columns[1].get()),
- assert_cast<const ColumnInt8*>(argument_columns[2].get()),
- assert_cast<ColumnString*>(result_column.get()),
-
assert_cast<ColumnUInt8*>(result_null_map_column.get())->get_data(),
- input_rows_count);
+ for (int i = 0; i < 3; i++) {
+ check_set_nullable(argument_columns[i], result_null_map_column);
+ }
+
+ if (col_const[1] && col_const[2]) {
+ execute_scalar_args(
+ context,
+ assert_cast<const typename Impl::DataType::ColumnType*>(
+ argument_columns[0].get()),
+ assert_cast<const
ColumnInt8*>(argument_columns[1].get())->get_element(0),
+ assert_cast<const
ColumnInt8*>(argument_columns[2].get())->get_element(0),
+ assert_cast<ColumnString*>(result_column.get()),
+
assert_cast<ColumnUInt8*>(result_null_map_column.get())->get_data(),
+ input_rows_count);
+ } else {
+ execute_straight(context,
+ assert_cast<const typename
Impl::DataType::ColumnType*>(
+ argument_columns[0].get()),
+ assert_cast<const
ColumnInt8*>(argument_columns[1].get()),
+ assert_cast<const
ColumnInt8*>(argument_columns[2].get()),
+ assert_cast<ColumnString*>(result_column.get()),
+
assert_cast<ColumnUInt8*>(result_null_map_column.get())->get_data(),
+ input_rows_count);
+ }
block.get_by_position(result).column =
ColumnNullable::create(std::move(result_column),
std::move(result_null_map_column));
return Status::OK();
}
private:
- void execute_straight(FunctionContext* context,
- const typename Impl::DataType::ColumnType*
data_column,
- const ColumnInt8* src_base_column, const ColumnInt8*
dst_base_column,
- ColumnString* result_column, NullMap&
result_null_map,
- size_t input_rows_count) {
+ static bool _check_set_oob(const Int8 src_base, const Int8 dst_base,
+ ColumnString* result_column, UInt8&
result_null) {
+ if (std::abs(src_base) < MathFunctions::MIN_BASE ||
+ std::abs(src_base) > MathFunctions::MAX_BASE ||
+ std::abs(dst_base) < MathFunctions::MIN_BASE ||
+ std::abs(dst_base) > MathFunctions::MAX_BASE) { //out of bound
+ result_null = true;
+ result_column->insert_default();
+ return true;
+ }
+ return false;
+ }
+ static void execute_straight(FunctionContext* context,
+ const typename Impl::DataType::ColumnType*
data_column,
+ const ColumnInt8* src_base_column,
+ const ColumnInt8* dst_base_column,
ColumnString* result_column,
+ NullMap& result_null_map, size_t
input_rows_count) {
for (size_t i = 0; i < input_rows_count; i++) {
if (result_null_map[i]) {
result_column->insert_default();
continue;
}
-
Int8 src_base = src_base_column->get_element(i);
Int8 dst_base = dst_base_column->get_element(i);
- if (std::abs(src_base) < MathFunctions::MIN_BASE ||
- std::abs(src_base) > MathFunctions::MAX_BASE ||
- std::abs(dst_base) < MathFunctions::MIN_BASE ||
- std::abs(dst_base) > MathFunctions::MAX_BASE) {
+ if (!_check_set_oob(src_base, dst_base, result_column,
result_null_map[i])) {
+ Impl::calculate_cell(context, data_column, src_base, dst_base,
result_column,
+ result_null_map, i);
+ }
+ }
+ }
+ static void execute_scalar_args(FunctionContext* context,
+ const typename Impl::DataType::ColumnType*
data_column,
+ const Int8 src_base, const Int8 dst_base,
+ ColumnString* result_column, NullMap&
result_null_map,
+ size_t input_rows_count) {
+ if (std::abs(src_base) < MathFunctions::MIN_BASE ||
+ std::abs(src_base) > MathFunctions::MAX_BASE ||
+ std::abs(dst_base) < MathFunctions::MIN_BASE ||
+ std::abs(dst_base) > MathFunctions::MAX_BASE) { //out of bound
+ for (int i = 0; i < input_rows_count; i++) {
result_null_map[i] = true;
result_column->insert_default();
Review Comment:
do not use for loop
```
memset->result_null_map
result_column->insert_many_default
```
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
