Backport #51301 to 23.4: Fix fuzzer failure in ActionsDAG

robot-clickhouse · robot-clickhouse · commit 005f1d0cf3a8 · 2023-06-27T15:05:21.000Z
diff --git a/src/Interpreters/ActionsDAG.cpp b/src/Interpreters/ActionsDAG.cpp
@@ -1874,10 +1874,10 @@ struct ConjunctionNodes
     ActionsDAG::NodeRawConstPtrs rejected;
 };
 
-/// Take a node which result is predicate.
+/// Take a node which result is a predicate.
 /// Assuming predicate is a conjunction (probably, trivial).
 /// Find separate conjunctions nodes. Split nodes into allowed and rejected sets.
-/// Allowed predicate is a predicate which can be calculated using only nodes from allowed_nodes set.
+/// Allowed predicate is a predicate which can be calculated using only nodes from the allowed_nodes set.
 ConjunctionNodes getConjunctionNodes(ActionsDAG::Node * predicate, std::unordered_set<const ActionsDAG::Node *> allowed_nodes)
 {
     ConjunctionNodes conjunction;
@@ -2111,9 +2111,9 @@ ActionsDAGPtr ActionsDAG::cloneActionsForFilterPushDown(
     Node * predicate = const_cast<Node *>(tryFindInOutputs(filter_name));
     if (!predicate)
         throw Exception(ErrorCodes::LOGICAL_ERROR,
-                "Output nodes for ActionsDAG do not contain filter column name {}. DAG:\n{}",
-                filter_name,
-                dumpDAG());
+            "Output nodes for ActionsDAG do not contain filter column name {}. DAG:\n{}",
+            filter_name,
+            dumpDAG());
 
     /// If condition is constant let's do nothing.
     /// It means there is nothing to push down or optimization was already applied.
@@ -2140,14 +2140,29 @@ ActionsDAGPtr ActionsDAG::cloneActionsForFilterPushDown(
     }
 
     auto conjunction = getConjunctionNodes(predicate, allowed_nodes);
-    if (conjunction.rejected.size() == 1 && WhichDataType{removeNullable(conjunction.rejected.front()->result_type)}.isFloat())
+
+    if (conjunction.allowed.empty())
         return nullptr;
 
+    chassert(predicate->result_type);
+
+    if (conjunction.rejected.size() == 1)
+    {
+        chassert(conjunction.rejected.front()->result_type);
+
+        if (conjunction.allowed.front()->type == ActionType::COLUMN
+            && !conjunction.rejected.front()->result_type->equals(*predicate->result_type))
+        {
+            /// No further optimization can be done
+            return nullptr;
+        }
+    }
+
     auto actions = cloneActionsForConjunction(conjunction.allowed, all_inputs);
     if (!actions)
         return nullptr;
 
-    /// Now, when actions are created, update current DAG.
+    /// Now, when actions are created, update the current DAG.
 
     if (conjunction.rejected.empty())
     {
diff --git a/tests/queries/0_stateless/02791_predicate_pushdown_different_types.reference b/tests/queries/0_stateless/02791_predicate_pushdown_different_types.reference
diff --git a/tests/queries/0_stateless/02791_predicate_pushdown_different_types.sql b/tests/queries/0_stateless/02791_predicate_pushdown_different_types.sql
@@ -0,0 +1,7 @@
+# These queries triggered a crash in old ClickHouse versions:
+
+CREATE TEMPORARY TABLE a (key UInt32, ID LowCardinality(String));
+CREATE TEMPORARY TABLE b (key UInt32);
+SELECT * FROM b JOIN a USING (key) WHERE ID = '1' HAVING ID = '1';
+
+# PS. Predicate pushdown does not work for LowCardinality(String), but it's another problem.
