From 3b6499cce96f6cd6f527aa67eff1d016c68231fb Mon Sep 17 00:00:00 2001 From: Alexander Kazakov Date: Sun, 19 Jan 2020 09:07:30 +0300 Subject: [PATCH 1/2] Made code in OperationApplier more generic --- dbms/src/Functions/FunctionsLogical.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dbms/src/Functions/FunctionsLogical.cpp b/dbms/src/Functions/FunctionsLogical.cpp index 6070ec735db..eed267c1102 100644 --- a/dbms/src/Functions/FunctionsLogical.cpp +++ b/dbms/src/Functions/FunctionsLogical.cpp @@ -283,29 +283,28 @@ template < struct OperationApplier { template - static void apply(Columns & in, ResultData & result_data) + static void apply(Columns & in, ResultData & result_data, bool use_result_data_as_input = false) { - /// TODO: Maybe reuse this code for constants (which may form precalculated result) - doBatchedApply(in, result_data); - + if (!use_result_data_as_input) + doBatchedApply(in, result_data); while (in.size() > 0) doBatchedApply(in, result_data); } - template + template static void NO_INLINE doBatchedApply(Columns & in, ResultData & result_data) { if (N > in.size()) { OperationApplier - ::template doBatchedApply(in, result_data); + ::template doBatchedApply(in, result_data); return; } const OperationApplierImpl operationApplierImpl(in); size_t i = 0; for (auto & res : result_data) - if constexpr (carryResult) + if constexpr (CarryResult) res = Op::apply(res, operationApplierImpl.apply(i++)); else res = operationApplierImpl.apply(i++); From bffd66fcba0141c92b59f5f20b2aa5896e03022e Mon Sep 17 00:00:00 2001 From: Alexander Kazakov Date: Sun, 19 Jan 2020 09:22:01 +0300 Subject: [PATCH 2/2] 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); }