From b6061e132aa704c099f6a93360ecd1ab02b2dff2 Mon Sep 17 00:00:00 2001 From: Pavel Kruglov Date: Tue, 27 Apr 2021 16:12:57 +0300 Subject: [PATCH] Small fixes --- src/Columns/ColumnArray.h | 12 ++++---- src/Columns/ColumnFunction.cpp | 1 - src/Columns/ColumnFunction.h | 1 - src/Columns/ColumnsCommon.h | 10 ++----- src/Columns/MaskOperations.h | 4 --- src/Functions/FunctionsLogical.cpp | 5 ++-- src/Functions/FunctionsLogical.h | 2 +- src/Interpreters/ActionsDAG.cpp | 38 +------------------------ src/Interpreters/ActionsDAG.h | 8 ++---- src/Interpreters/ActionsVisitor.cpp | 4 +-- src/Interpreters/ExpressionActions.cpp | 2 +- src/Interpreters/ExpressionAnalyzer.cpp | 5 ++-- 12 files changed, 21 insertions(+), 71 deletions(-) diff --git a/src/Columns/ColumnArray.h b/src/Columns/ColumnArray.h index abf63baf669..888472c4dd0 100644 --- a/src/Columns/ColumnArray.h +++ b/src/Columns/ColumnArray.h @@ -70,7 +70,7 @@ public: void insertFrom(const IColumn & src_, size_t n) override; void insertDefault() override; void popBack(size_t n) override; - ColumnPtr filter(const Filter & filt, ssize_t result_size_hint, bool revers) const override; + ColumnPtr filter(const Filter & filt, ssize_t result_size_hint, bool reverse) const override; void expand(const Filter & mask, bool reverse) override; ColumnPtr permute(const Permutation & perm, size_t limit) const override; ColumnPtr index(const IColumn & indexes, size_t limit) const override; @@ -174,12 +174,12 @@ private: /// Specializations for the filter function. template - ColumnPtr filterNumber(const Filter & filt, ssize_t result_size_hint, bool reverse = false) const; + ColumnPtr filterNumber(const Filter & filt, ssize_t result_size_hint, bool reverse) const; - ColumnPtr filterString(const Filter & filt, ssize_t result_size_hint, bool reverse = false) const; - ColumnPtr filterTuple(const Filter & filt, ssize_t result_size_hint, bool reverse = false) const; - ColumnPtr filterNullable(const Filter & filt, ssize_t result_size_hint, bool reverse = false) const; - ColumnPtr filterGeneric(const Filter & filt, ssize_t result_size_hint, bool reverse = false) const; + ColumnPtr filterString(const Filter & filt, ssize_t result_size_hint, bool reverse) const; + ColumnPtr filterTuple(const Filter & filt, ssize_t result_size_hint, bool reverse) const; + ColumnPtr filterNullable(const Filter & filt, ssize_t result_size_hint, bool reverse) const; + ColumnPtr filterGeneric(const Filter & filt, ssize_t result_size_hint, bool reverse) const; int compareAtImpl(size_t n, size_t m, const IColumn & rhs_, int nan_direction_hint, const Collator * collator=nullptr) const; diff --git a/src/Columns/ColumnFunction.cpp b/src/Columns/ColumnFunction.cpp index fcffdf32fb4..b83dcda5c88 100644 --- a/src/Columns/ColumnFunction.cpp +++ b/src/Columns/ColumnFunction.cpp @@ -5,7 +5,6 @@ #include #include -#include namespace DB { diff --git a/src/Columns/ColumnFunction.h b/src/Columns/ColumnFunction.h index bfada4775ea..895f61ec6db 100644 --- a/src/Columns/ColumnFunction.h +++ b/src/Columns/ColumnFunction.h @@ -158,7 +158,6 @@ private: size_t size_; FunctionBasePtr function; ColumnsWithTypeAndName captured_columns; - bool is_short_circuit_argumentz; void appendArgument(const ColumnWithTypeAndName & column); }; diff --git a/src/Columns/ColumnsCommon.h b/src/Columns/ColumnsCommon.h index 97998196523..76eb7525f56 100644 --- a/src/Columns/ColumnsCommon.h +++ b/src/Columns/ColumnsCommon.h @@ -32,7 +32,7 @@ template void filterArraysImpl( const PaddedPODArray & src_elems, const IColumn::Offsets & src_offsets, PaddedPODArray & res_elems, IColumn::Offsets & res_offsets, - const IColumn::Filter & filt, ssize_t result_size_hint, bool reverse = false); + const IColumn::Filter & filt, ssize_t result_size_hint, bool reverse); /// Same as above, but not fills res_offsets. template @@ -41,11 +41,6 @@ void filterArraysImplOnlyData( PaddedPODArray & res_elems, const IColumn::Filter & filt, ssize_t result_size_hint, bool reverse = false); -template -void expandDataByMask(Container & data, const PaddedPODArray & mask, bool reverse, T default_value); - -void expandOffsetsByMask(PaddedPODArray & offsets, const PaddedPODArray & mask, bool reverse); - namespace detail { template @@ -71,11 +66,10 @@ ColumnPtr selectIndexImpl(const Column & column, const IColumn & indexes, size_t else if (auto * data_uint64 = detail::getIndexesData(indexes)) return column.template indexImpl(*data_uint64, limit); else - throw Exception("Indexes column for IColumn::select must be ColumnUInt, got" + indexes.getName(), + throw Exception("Indexes column for IColumn::select must be ColumnUInt, got " + indexes.getName(), ErrorCodes::LOGICAL_ERROR); } - #define INSTANTIATE_INDEX_IMPL(Column) \ template ColumnPtr Column::indexImpl(const PaddedPODArray & indexes, size_t limit) const; \ template ColumnPtr Column::indexImpl(const PaddedPODArray & indexes, size_t limit) const; \ diff --git a/src/Columns/MaskOperations.h b/src/Columns/MaskOperations.h index ff208153107..10bc4472641 100644 --- a/src/Columns/MaskOperations.h +++ b/src/Columns/MaskOperations.h @@ -21,10 +21,6 @@ void disjunctionMasks(PaddedPODArray & mask1, const PaddedPODArray void maskedExecute(ColumnWithTypeAndName & column, const PaddedPODArray & mask, bool reverse = false, const UInt8 * default_value_for_expanding_mask = nullptr); -void expandColumnByMask(const ColumnPtr & column, const PaddedPODArray& mask, bool reverse); - -void expandMaskColumnByMask(const ColumnPtr & column, const PaddedPODArray& mask, bool reverse, UInt8 default_value = 0); - void executeColumnIfNeeded(ColumnWithTypeAndName & column); bool checkArgumentsForColumnFunction(const ColumnsWithTypeAndName & arguments); diff --git a/src/Functions/FunctionsLogical.cpp b/src/Functions/FunctionsLogical.cpp index 6253a784f09..d55dc3cc758 100644 --- a/src/Functions/FunctionsLogical.cpp +++ b/src/Functions/FunctionsLogical.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -14,8 +15,6 @@ #include -#include -#include namespace DB { @@ -475,7 +474,7 @@ static ColumnPtr basicExecuteImpl(ColumnRawPtrs arguments, size_t input_rows_cou } template - DataTypePtr FunctionAnyArityLogical::getReturnTypeImpl(const DataTypes & arguments) const +DataTypePtr FunctionAnyArityLogical::getReturnTypeImpl(const DataTypes & arguments) const { if (arguments.size() < 2) throw Exception("Number of arguments for function \"" + getName() + "\" should be at least 2: passed " diff --git a/src/Functions/FunctionsLogical.h b/src/Functions/FunctionsLogical.h index 339e8d46429..8a3a9defdc3 100644 --- a/src/Functions/FunctionsLogical.h +++ b/src/Functions/FunctionsLogical.h @@ -30,8 +30,8 @@ */ namespace DB - { + struct NameAnd { static constexpr auto name = "and"; }; struct NameOr { static constexpr auto name = "or"; }; struct NameXor { static constexpr auto name = "xor"; }; diff --git a/src/Interpreters/ActionsDAG.cpp b/src/Interpreters/ActionsDAG.cpp index 72b23b8bf9a..63b0345b372 100644 --- a/src/Interpreters/ActionsDAG.cpp +++ b/src/Interpreters/ActionsDAG.cpp @@ -1,6 +1,5 @@ #include -#include #include #include #include @@ -174,8 +173,7 @@ const ActionsDAG::Node & ActionsDAG::addArrayJoin(const Node & child, std::strin const ActionsDAG::Node & ActionsDAG::addFunction( const FunctionOverloadResolverPtr & function, NodeRawConstPtrs children, - std::string result_name, - bool use_short_circuit_function_evaluation) + std::string result_name) { size_t num_arguments = children.size(); @@ -250,36 +248,9 @@ const ActionsDAG::Node & ActionsDAG::addFunction( node.result_name = std::move(result_name); - if (node.function_base->isShortCircuit() && use_short_circuit_function_evaluation) - rewriteShortCircuitArguments(node.children, 1); - return addNode(std::move(node)); } -void ActionsDAG::rewriteShortCircuitArguments(const NodeRawConstPtrs & children, size_t start) -{ - for (size_t i = start; i < children.size(); ++i) - { - switch (children[i]->type) - { - case ActionType::FUNCTION: - { - Node * node = const_cast(children[i]); - node->type = ActionType::COLUMN_FUNCTION; - rewriteShortCircuitArguments(node->children); - break; - } - case ActionType::ALIAS: - { - rewriteShortCircuitArguments(children[i]->children); - break; - } - default: - break; - } - } -} - const ActionsDAG::Node & ActionsDAG::findInIndex(const std::string & name) const { if (const auto * node = tryFindInIndex(name)) @@ -963,10 +934,6 @@ std::string ActionsDAG::dumpDAG() const case ActionsDAG::ActionType::INPUT: out << "INPUT "; break; - - case ActionsDAG::ActionType::COLUMN_FUNCTION: - out << "COLUMN FUNCTION"; - break; } out << "("; @@ -1578,9 +1545,6 @@ ActionsDAG::SplitResult ActionsDAG::splitActionsForFilter(const std::string & co "Index for ActionsDAG does not contain filter column name {}. DAG:\n{}", column_name, dumpDAG()); - if (node->type == ActionType::COLUMN_FUNCTION) - const_cast(node)->type = ActionType::FUNCTION; - std::unordered_set split_nodes = {node}; auto res = split(split_nodes); res.second->project_input = project_input; diff --git a/src/Interpreters/ActionsDAG.h b/src/Interpreters/ActionsDAG.h index ea097e691d7..77fa3ed7be3 100644 --- a/src/Interpreters/ActionsDAG.h +++ b/src/Interpreters/ActionsDAG.h @@ -59,7 +59,6 @@ public: /// Function arrayJoin. Specially separated because it changes the number of rows. ARRAY_JOIN, FUNCTION, - COLUMN_FUNCTION, }; static const char * typeToString(ActionType type); @@ -92,6 +91,8 @@ public: ColumnPtr column; void toTree(JSONBuilder::JSONMap & map) const; + + bool is_lazy_executed = false; }; /// NOTE: std::list is an implementation detail. @@ -136,8 +137,7 @@ public: const Node & addFunction( const FunctionOverloadResolverPtr & function, NodeRawConstPtrs children, - std::string result_name, - bool use_short_circuit_function_evaluation = false); + std::string result_name); /// Index can contain any column returned from DAG. /// You may manually change it if needed. @@ -277,8 +277,6 @@ private: void removeUnusedActions(bool allow_remove_inputs = true); - void rewriteShortCircuitArguments(const NodeRawConstPtrs & children, size_t start = 0); - #if USE_EMBEDDED_COMPILER void compileFunctions(size_t min_count_to_compile_expression); #endif diff --git a/src/Interpreters/ActionsVisitor.cpp b/src/Interpreters/ActionsVisitor.cpp index 3734d15fcfa..61e484ff6f1 100644 --- a/src/Interpreters/ActionsVisitor.cpp +++ b/src/Interpreters/ActionsVisitor.cpp @@ -566,8 +566,8 @@ void ScopeStack::addFunction( children.reserve(argument_names.size()); for (const auto & argument : argument_names) children.push_back(&stack[level].index->getNode(argument)); - const auto & node = stack[level].actions_dag->addFunction( - function, std::move(children), std::move(result_name), getContext()->getSettingsRef().use_short_circuit_function_evaluation); + + const auto & node = stack[level].actions_dag->addFunction(function, std::move(children), std::move(result_name)); stack[level].index->addNode(&node); for (size_t j = level + 1; j < stack.size(); ++j) diff --git a/src/Interpreters/ExpressionActions.cpp b/src/Interpreters/ExpressionActions.cpp index 0026d87031e..6fb3dbbb20a 100644 --- a/src/Interpreters/ExpressionActions.cpp +++ b/src/Interpreters/ExpressionActions.cpp @@ -388,7 +388,7 @@ namespace ColumnsWithTypeAndName & inputs; ColumnsWithTypeAndName columns = {}; std::vector inputs_pos = {}; - size_t num_rows; + size_t num_rows = 0; }; } diff --git a/src/Interpreters/ExpressionAnalyzer.cpp b/src/Interpreters/ExpressionAnalyzer.cpp index 1d4b2e6a065..77598e69c00 100644 --- a/src/Interpreters/ExpressionAnalyzer.cpp +++ b/src/Interpreters/ExpressionAnalyzer.cpp @@ -88,7 +88,7 @@ bool allowEarlyConstantFolding(const ActionsDAG & actions, const Settings & sett for (const auto & node : actions.getNodes()) { - if ((node.type == ActionsDAG::ActionType::FUNCTION) && node.function_base) + if (node.type == ActionsDAG::ActionType::FUNCTION && node.function_base) { if (!node.function_base->isSuitableForConstantFolding()) return false; @@ -1578,7 +1578,8 @@ ExpressionAnalysisResult::ExpressionAnalysisResult( optimize_read_in_order = settings.optimize_read_in_order - && storage && query.orderBy() + && storage + && query.orderBy() && !query_analyzer.hasAggregation() && !query_analyzer.hasWindow() && !query.final()