fix

src/Interpreters/ComparisonGraph.cpp

@@ -12,7 +12,7 @@ namespace DB
 {
 
 /// make function a < b or a <= b
-ASTPtr ComparisonGraph::normalizeAtom(const ASTPtr & atom) const
+ASTPtr ComparisonGraph::normalizeAtom(const ASTPtr & atom)
 {
     static const std::map<std::string, std::string> inverse_relations = {
         {"greaterOrEquals", "lessOrEquals"},
@@ -46,7 +46,7 @@ ComparisonGraph::ComparisonGraph(const std::vector<ASTPtr> & atomic_formulas)
     Graph g;
     for (const auto & atom_raw : atomic_formulas)
     {
-        const auto atom = normalizeAtom(atom_raw);
+        const auto atom = ComparisonGraph::normalizeAtom(atom_raw);
 
         const auto bad_term = std::numeric_limits<std::size_t>::max();
         auto get_index = [](const ASTPtr & ast, Graph & asts_graph) -> std::size_t
@@ -103,12 +103,12 @@ ComparisonGraph::ComparisonGraph(const std::vector<ASTPtr> & atomic_formulas)
         }
     }
 
-    graph = BuildGraphFromAstsGraph(g);
+    graph = ComparisonGraph::BuildGraphFromAstsGraph(g);
-    dists = BuildDistsFromGraph(graph);
+    dists = ComparisonGraph::BuildDistsFromGraph(graph);
     std::tie(ast_const_lower_bound, ast_const_upper_bound) = buildConstBounds();
 }
 
-/// resturns {is less, is strict}
+/// returns {is less, is strict}
 /// {true, true} = <
 /// {true, false} = =<
 /// {false, ...} = ?
@@ -199,7 +199,7 @@ bool ComparisonGraph::isPossibleCompare(const CompareResult expected, const ASTP
 
     if (expected == CompareResult::UNKNOWN || result == CompareResult::UNKNOWN)
     {
-        Poco::Logger::get("isPossibleCompare").information("unknonw");
+        Poco::Logger::get("isPossibleCompare").information("unknown");
         return true;
     }
     if (expected == result)
@@ -392,7 +392,7 @@ std::optional<std::pair<Field, bool>> ComparisonGraph::getConstLowerBound(const
     return std::make_pair(graph.vertices[to].getConstant()->as<ASTLiteral>()->value, dists.at({from, to}) == Path::LESS);
 }
 
-void ComparisonGraph::dfsOrder(const Graph & asts_graph, size_t v, std::vector<bool> & visited, std::vector<size_t> & order) const
+void ComparisonGraph::dfsOrder(const Graph & asts_graph, size_t v, std::vector<bool> & visited, std::vector<size_t> & order)
 {
     visited[v] = true;
     for (const auto & edge : asts_graph.edges[v])
@@ -405,7 +405,7 @@ void ComparisonGraph::dfsOrder(const Graph & asts_graph, size_t v, std::vector<b
     order.push_back(v);
 }
 
-ComparisonGraph::Graph ComparisonGraph::reverseGraph(const Graph & asts_graph) const
+ComparisonGraph::Graph ComparisonGraph::reverseGraph(const Graph & asts_graph)
 {
     Graph g;
     g.ast_hash_to_component = asts_graph.ast_hash_to_component;
@@ -434,7 +434,7 @@ std::vector<ASTs> ComparisonGraph::getVertices() const
 }
 
 void ComparisonGraph::dfsComponents(
-    const Graph & reversed_graph, size_t v, std::vector<size_t> & components, const size_t not_visited, const size_t component) const
+    const Graph & reversed_graph, size_t v, std::vector<size_t> & components, const size_t not_visited, const size_t component)
 {
     components[v] = component;
     for (const auto & edge : reversed_graph.edges[v])
@@ -446,7 +446,7 @@ void ComparisonGraph::dfsComponents(
     }
 }
 
-ComparisonGraph::Graph ComparisonGraph::BuildGraphFromAstsGraph(const Graph & asts_graph) const
+ComparisonGraph::Graph ComparisonGraph::BuildGraphFromAstsGraph(const Graph & asts_graph)
 {
     Poco::Logger::get("Graph").information("building");
     /// Find strongly connected component
@@ -458,7 +458,7 @@ ComparisonGraph::Graph ComparisonGraph::BuildGraphFromAstsGraph(const Graph & as
         for (size_t v = 0; v < n; ++v)
         {
             if (!visited[v])
-                dfsOrder(asts_graph, v, visited, order);
+                ComparisonGraph::dfsOrder(asts_graph, v, visited, order);
         }
     }
 
@@ -471,7 +471,7 @@ ComparisonGraph::Graph ComparisonGraph::BuildGraphFromAstsGraph(const Graph & as
         {
             if (components[v] == not_visited)
             {
-                dfsComponents(reversed_graph, v, components, not_visited, component);
+                ComparisonGraph::dfsComponents(reversed_graph, v, components, not_visited, component);
                 ++component;
             }
         }
@@ -529,11 +529,11 @@ ComparisonGraph::Graph ComparisonGraph::BuildGraphFromAstsGraph(const Graph & as
     return result;
 }
 
-std::map<std::pair<size_t, size_t>, ComparisonGraph::Path> ComparisonGraph::BuildDistsFromGraph(const Graph & g) const
+std::map<std::pair<size_t, size_t>, ComparisonGraph::Path> ComparisonGraph::BuildDistsFromGraph(const Graph & g)
 {
     // min path : < = -1, =< = 0
     const auto inf = std::numeric_limits<int8_t>::max();
-    const size_t n = graph.vertices.size();
+    const size_t n = g.vertices.size();
     std::vector<std::vector<int8_t>> results(n, std::vector<int8_t>(n, inf));
     for (size_t v = 0; v < n; ++v)
     {

src/Interpreters/ComparisonGraph.h

@@ -99,13 +99,13 @@ private:
         std::vector<std::vector<Edge>> edges;
     };
 
-    ASTPtr normalizeAtom(const ASTPtr & atom) const;
+    static ASTPtr normalizeAtom(const ASTPtr & atom);
-    Graph BuildGraphFromAstsGraph(const Graph & asts_graph) const;
+    static Graph BuildGraphFromAstsGraph(const Graph & asts_graph);
 
-    Graph reverseGraph(const Graph & asts_graph) const;
+    static Graph reverseGraph(const Graph & asts_graph);
-    void dfsOrder(const Graph & asts_graph, size_t v, std::vector<bool> & visited, std::vector<size_t> & order) const;
+    static void dfsOrder(const Graph & asts_graph, size_t v, std::vector<bool> & visited, std::vector<size_t> & order);
-    void dfsComponents(
+    static void dfsComponents(
-            const Graph & reversed_graph, size_t v, std::vector<size_t> & components, const size_t not_visited, const size_t component) const;
+            const Graph & reversed_graph, size_t v, std::vector<size_t> & components, const size_t not_visited, const size_t component);
 
     std::pair<bool, bool> findPath(const size_t start, const size_t finish) const;
 
@@ -115,7 +115,7 @@ private:
         LESS_OR_EQUAL,
     };
 
-    std::map<std::pair<size_t, size_t>, Path> BuildDistsFromGraph(const Graph & g) const;
+    static std::map<std::pair<size_t, size_t>, Path> BuildDistsFromGraph(const Graph & g);
     std::pair<std::vector<ssize_t>, std::vector<ssize_t>> buildConstBounds() const;
 
     Graph graph;

src/Interpreters/TreeCNFConverter.cpp

@@ -5,6 +5,10 @@
 
 namespace DB
 {
+namespace ErrorCodes
+{
+    extern const int LOGICAL_ERROR;
+}
 
 /// Splits AND(a, b, c) to AND(a, AND(b, c)) for AND/OR
 void splitMultiLogic(ASTPtr & node)
@@ -38,7 +42,6 @@ void traversePushNot(ASTPtr & node, bool add_negation)
     {
         if (add_negation)
         {
-            ASSERT(func->arguments->size() == 2)
             if (func->arguments->children.size() != 2)
                 throw Exception("Bad AND or OR function.", ErrorCodes::LOGICAL_ERROR);
             /// apply De Morgan's Law
@@ -54,7 +57,6 @@ void traversePushNot(ASTPtr & node, bool add_negation)
     }
     else if (func && func->name == "not")
     {
-        ASSERT(func->arguments->size() == 1)
         if (func->arguments->children.size() != 1)
             throw Exception("Bad NOT function.", ErrorCodes::LOGICAL_ERROR);
         /// delete NOT
@@ -95,9 +97,6 @@ void pushOr(ASTPtr & query)
         ors.pop_back();
 
         auto * or_func = or_node.get()->as<ASTFunction>();
-        ASSERT(or_func)
-        ASSERT(or_func->name == "or")
-        ASSERT(or_func->arguments->children.size() == 2)
         if (or_func->arguments->children.size() != 2)
             throw Exception("Bad OR function.", ErrorCodes::LOGICAL_ERROR);
 
@@ -116,10 +115,7 @@ void pushOr(ASTPtr & query)
             continue;
         const size_t other_node_id = 1 - and_node_id;
 
-        auto and_func = or_func->arguments->children[and_node_id]->as<ASTFunction>();
+        const auto * and_func = or_func->arguments->children[and_node_id]->as<ASTFunction>();
-        ASSERT(and_func)
-        ASSERT(and_func->name == "and")
-        ASSERT(and_func->arguments->children.size() == 2)
         if (and_func->arguments->children.size() != 2)
             throw Exception("Bad AND function.", ErrorCodes::LOGICAL_ERROR);
 

src/Storages/ConstraintsDescription.cpp

@@ -14,6 +14,10 @@
 
 namespace DB
 {
+namespace ErrorCodes
+{
+    extern const int LOGICAL_ERROR;
+}
 
 String ConstraintsDescription::toString() const
 {
@@ -107,17 +111,18 @@ std::unique_ptr<ComparisonGraph> ConstraintsDescription::buildGraph() const
 
     std::vector<ASTPtr> constraints_for_graph;
     auto atomic_formulas = getAtomicConstraintData();
-    for (auto & atomic_formula : atomic_formulas)
+    for (const auto & atomic_formula : atomic_formulas)
     {
         Poco::Logger::get("atomic_formula: before:").information(atomic_formula.ast->dumpTree() + " " + std::to_string(atomic_formula.negative));
-        pushNotIn(atomic_formula);
+        CNFQuery::AtomicFormula atom{atomic_formula.negative, atomic_formula.ast->clone()};
-        auto * func = atomic_formula.ast->as<ASTFunction>();
+        pushNotIn(atom);
+        auto * func = atom.ast->as<ASTFunction>();
         if (func && relations.count(func->name))
         {
-            if (atomic_formula.negative)
+            if (atom.negative)
                 throw Exception(": ", ErrorCodes::LOGICAL_ERROR);
-            Poco::Logger::get("atomic_formula: after:").information(atomic_formula.ast->dumpTree() + " " + std::to_string(atomic_formula.negative));
+            Poco::Logger::get("atomic_formula: after:").information(atom.ast->dumpTree() + " " + std::to_string(atom.negative));
-            constraints_for_graph.push_back(atomic_formula.ast);
+            constraints_for_graph.push_back(atom.ast);
         }
     }
 

src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp

@@ -621,7 +621,7 @@ QueryPlanPtr MergeTreeDataSelectExecutor::readFromParts(
         std::atomic<size_t> total_parts{0};
         std::atomic<size_t> parts_dropped{0};
 
-        MergedDataSkippingIndexAndCondition(MergeTreeIndexMergedConditionPtr condition_)
+        explicit MergedDataSkippingIndexAndCondition(MergeTreeIndexMergedConditionPtr condition_)
             : condition(condition_)
         {
         }

src/Storages/MergeTree/MergeTreeDataSelectExecutor.h

@@ -127,7 +127,7 @@ private:
         Poco::Logger * log);
 
     static MarkRanges filterMarksUsingMergedIndex(
-        MergeTreeIndices index_helper,
+        MergeTreeIndices indices,
         MergeTreeIndexMergedConditionPtr condition,
         MergeTreeData::DataPartPtr part,
         const MarkRanges & ranges,

src/Storages/MergeTree/MergeTreeIndexHypothesis.cpp

@@ -14,7 +14,6 @@ namespace DB
 namespace ErrorCodes
 {
     extern const int LOGICAL_ERROR;
-    extern const int INCORRECT_QUERY;
 }
 
 

src/Storages/MergeTree/MergeTreeIndexMergedCondition.cpp

@@ -59,7 +59,8 @@ void MergeTreeIndexMergedCondition::addIndex(const MergeTreeIndexPtr & index)
         if (group.size() == 1)
         {
             hypotheses_data.push_back(group);
-            CNFQuery::AtomicFormula atom = *group.begin();
+            CNFQuery::AtomicFormula atomic_formula = *group.begin();
+            CNFQuery::AtomicFormula atom{atomic_formula.negative, atomic_formula.ast->clone()};
             pushNotIn(atom);
             if (atom.negative)
                 throw Exception("negative atom", ErrorCodes::LOGICAL_ERROR);
@@ -76,8 +77,9 @@ void MergeTreeIndexMergedCondition::addIndex(const MergeTreeIndexPtr & index)
 void MergeTreeIndexMergedCondition::addConstraints(const ConstraintsDescription & constraints_description)
 {
     auto atomic_constraints_data = constraints_description.getAtomicConstraintData();
-    for (auto & atom : atomic_constraints_data)
+    for (const auto & atomic_formula : atomic_constraints_data)
     {
+        CNFQuery::AtomicFormula atom{atomic_formula.negative, atomic_formula.ast->clone()};
         pushNotIn(atom);
         atomic_constraints.push_back(atom.ast);
     }
@@ -105,12 +107,12 @@ ComparisonGraph::CompareResult getExpectedCompare(const CNFQuery::AtomicFormula
 bool MergeTreeIndexMergedCondition::alwaysUnknownOrTrue() const
 {
     std::vector<ASTPtr> active_atomic_formulas(atomic_constraints);
-    for (size_t i = 0; i < index_to_compare_atomic_hypotheses.size(); ++i)
+    for (const auto & hypothesis : index_to_compare_atomic_hypotheses)
     {
         active_atomic_formulas.insert(
             std::end(active_atomic_formulas),
-            std::begin(index_to_compare_atomic_hypotheses[i]),
+            std::begin(hypothesis),
-            std::end(index_to_compare_atomic_hypotheses[i]));
+            std::end(hypothesis));
     }
 
     /// transform active formulas
@@ -130,8 +132,9 @@ bool MergeTreeIndexMergedCondition::alwaysUnknownOrTrue() const
     expression_cnf->iterateGroups(
         [&](const CNFQuery::OrGroup & or_group)
         {
-            for (auto atom : or_group)
+            for (const auto & atomic_formula : or_group)
             {
+                CNFQuery::AtomicFormula atom{atomic_formula.negative, atomic_formula.ast->clone()};
                 pushNotIn(atom);
                 const auto * func = atom.ast->as<ASTFunction>();
                 if (func && func->arguments->children.size() == 2)
@@ -168,8 +171,9 @@ bool MergeTreeIndexMergedCondition::mayBeTrueOnGranule(const MergeTreeIndexGranu
             if (always_false)
                 return;
 
-            for (auto atom : or_group)
+            for (const auto & atomic_formula : or_group)
             {
+                CNFQuery::AtomicFormula atom{atomic_formula.negative, atomic_formula.ast->clone()};
                 pushNotIn(atom);
                 Poco::Logger::get("KEK").information(atom.ast->dumpTree());
                 const auto * func = atom.ast->as<ASTFunction>();

src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp

@@ -192,7 +192,7 @@ void MergeTreeWhereOptimizer::optimize(ASTSelectQuery & select) const
         Poco::Logger::get("MTPRWHERE WHERE").information(select.where()->getColumnName());
         Poco::Logger::get("MTPRWHERE WHERE").information(select.where()->dumpTree());
     }
-    if(select.prewhere())
+    if (select.prewhere())
     {
         Poco::Logger::get("MTPRWHERE PRE").information(select.prewhere()->dumpTree());
         Poco::Logger::get("MTPRWHERE PRE").information(select.prewhere()->getColumnName());

src/Storages/MergeTree/SubstituteColumnOptimizer.cpp

@@ -13,6 +13,10 @@
 
 namespace DB
 {
+namespace ErrorCodes
+{
+    extern const int LOGICAL_ERROR;
+}
 
 namespace
 {
@@ -140,7 +144,7 @@ public:
         const auto * identifier = ast->as<ASTIdentifier>();
         if (identifier && data.name_to_component_id.contains(identifier->name()))
         {
-            const auto name = identifier->name();
+            const auto & name = identifier->name();
             const auto component_id = data.name_to_component_id.at(name);
             ast = data.id_to_expression_map.at(component_id)->clone();
             if (data.is_select)