From bffd66fcba0141c92b59f5f20b2aa5896e03022e Mon Sep 17 00:00:00 2001 From: Alexander Kazakov Date: Sun, 19 Jan 2020 09:22:01 +0300 Subject: [PATCH] More effective code + some cleanup --- dbms/src/Functions/FunctionsLogical.cpp | 46 ++++++------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/dbms/src/Functions/FunctionsLogical.cpp b/dbms/src/Functions/FunctionsLogical.cpp index eed267c1102..b5c64ce9602 100644 --- a/dbms/src/Functions/FunctionsLogical.cpp +++ b/dbms/src/Functions/FunctionsLogical.cpp @@ -344,16 +344,10 @@ static void executeForTernaryLogicImpl(ColumnRawPtrs arguments, ColumnWithTypeAn return; } - const auto result_column = ColumnUInt8::create(input_rows_count); - MutableColumnPtr const_column_holder; - if (has_consts) - { - const_column_holder = - convertFromTernaryData(UInt8Container(input_rows_count, const_3v_value), const_3v_value == Ternary::Null); - arguments.push_back(const_column_holder.get()); - } + const auto result_column = has_consts ? + ColumnUInt8::create(input_rows_count, const_3v_value) : ColumnUInt8::create(input_rows_count); - OperationApplier::apply(arguments, result_column->getData()); + OperationApplier::apply(arguments, result_column->getData(), has_consts); result_info.column = convertFromTernaryData(result_column->getData(), result_info.type->isNullable()); } @@ -428,32 +422,24 @@ static void basicExecuteImpl(ColumnRawPtrs arguments, ColumnWithTypeAndName & re if (has_consts && Op::apply(const_val, 0) == 0 && Op::apply(const_val, 1) == 1) has_consts = false; - auto col_res = ColumnUInt8::create(input_rows_count, const_val); - UInt8Container & vec_res = col_res->getData(); + auto col_res = has_consts ? + ColumnUInt8::create(input_rows_count, const_val) : ColumnUInt8::create(input_rows_count); /// FastPath detection goes in here if (arguments.size() == (has_consts ? 1 : 2)) { if (has_consts) - FastApplierImpl::apply(*arguments[0], *col_res, vec_res); + FastApplierImpl::apply(*arguments[0], *col_res, col_res->getData()); else - FastApplierImpl::apply(*arguments[0], *arguments[1], vec_res); + FastApplierImpl::apply(*arguments[0], *arguments[1], col_res->getData()); result_info.column = std::move(col_res); return; } - UInt8ColumnPtrs uint8_args; - if (has_consts) - { - /// TODO: This will FAIL (or not =) ) after correction b/c we now overwrite - /// the result column in the first pass of OperationApplier - // vec_res.assign(input_rows_count, const_val); - uint8_args.push_back(col_res.get()); - } - /// Convert all columns to UInt8 - Columns converted_columns; + UInt8ColumnPtrs uint8_args; + Columns converted_columns_holder; for (const IColumn * column : arguments) { if (auto uint8_column = checkAndGetColumn(column)) @@ -463,21 +449,11 @@ static void basicExecuteImpl(ColumnRawPtrs arguments, ColumnWithTypeAndName & re auto converted_column = ColumnUInt8::create(input_rows_count); convertColumnToUInt8(column, converted_column->getData()); uint8_args.push_back(converted_column.get()); - converted_columns.emplace_back(std::move(converted_column)); + converted_columns_holder.emplace_back(std::move(converted_column)); } } - OperationApplier::apply(uint8_args, col_res->getData()); - - /// TODO: The following code is obsolete and is to be removed now - /// - /// This is possible if there is exactly one non-constant among the arguments, and it is of type UInt8. - /// Suppose we have all constants folded into a neutral value and there is only one non-constant column. - /// Although not all logical functions have a neutral value. - /* - if (uint8_args[0] != col_res.get()) - vec_res.assign(uint8_args[0]->getData()); - */ + OperationApplier::apply(uint8_args, col_res->getData(), has_consts); result_info.column = std::move(col_res); }