From 15f20cb4e87531e364bc4cd5e34f745fd5387245 Mon Sep 17 00:00:00 2001
From: Dmitry Novik <n0vik@clickhouse.com>
Date: Tue, 3 Jan 2023 18:13:31 +0000
Subject: [PATCH] Fix column comparison

---
 ...ptimizeRedundantFunctionsInOrderByPass.cpp | 10 +++----
 ..._redundant_functions_in_order_by.reference | 28 +++++++++++++++++++
 .../01323_redundant_functions_in_order_by.sql |  9 ++++++
 3 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/src/Analyzer/Passes/OptimizeRedundantFunctionsInOrderByPass.cpp b/src/Analyzer/Passes/OptimizeRedundantFunctionsInOrderByPass.cpp
index 8136052cbd5..105fc0ef00a 100644
--- a/src/Analyzer/Passes/OptimizeRedundantFunctionsInOrderByPass.cpp
+++ b/src/Analyzer/Passes/OptimizeRedundantFunctionsInOrderByPass.cpp
@@ -1,7 +1,7 @@
-#include <string_view>
 #include <Analyzer/Passes/OptimizeRedundantFunctionsInOrderByPass.h>
 #include <Analyzer/ColumnNode.h>
 #include <Analyzer/FunctionNode.h>
+#include <Analyzer/HashUtils.h>
 #include <Analyzer/InDepthQueryTreeVisitor.h>
 #include <Analyzer/QueryNode.h>
 #include <Analyzer/SortNode.h>
@@ -56,8 +56,7 @@ public:
                 }
                 case QueryTreeNodeType::COLUMN:
                 {
-                    auto * column = order_by_expr->as<ColumnNode>();
-                    existing_keys.insert(column->getColumnName());
+                    existing_keys.insert(order_by_expr);
                     break;
                 }
                 default:
@@ -73,7 +72,7 @@ public:
     }
 
 private:
-    std::unordered_set<std::string_view> existing_keys;
+    QueryTreeNodePtrWithHashSet existing_keys;
 
     bool isRedundantExpression(QueryTreeNodePtr function)
     {
@@ -103,8 +102,7 @@ private:
                 }
                 case QueryTreeNodeType::COLUMN:
                 {
-                    auto * column = node->as<ColumnNode>();
-                    if (!existing_keys.contains(column->getColumnName()))
+                    if (!existing_keys.contains(node))
                         return false;
                     break;
                 }
diff --git a/tests/queries/0_stateless/01323_redundant_functions_in_order_by.reference b/tests/queries/0_stateless/01323_redundant_functions_in_order_by.reference
index ae160ed35d6..c69f8bb2c46 100644
--- a/tests/queries/0_stateless/01323_redundant_functions_in_order_by.reference
+++ b/tests/queries/0_stateless/01323_redundant_functions_in_order_by.reference
@@ -279,6 +279,34 @@ QUERY id: 0
       SORT id: 10, sort_direction: ASCENDING, with_fill: 0
         EXPRESSION
           COLUMN id: 2, column_name: key, result_type: UInt64, source_id: 3
+QUERY id: 0
+  PROJECTION COLUMNS
+    t1.id UInt64
+    t2.id UInt64
+  PROJECTION
+    LIST id: 1, nodes: 2
+      COLUMN id: 2, column_name: id, result_type: UInt64, source_id: 3
+      COLUMN id: 4, column_name: id, result_type: UInt64, source_id: 5
+  JOIN TREE
+    JOIN id: 6, strictness: ALL, kind: INNER
+      LEFT TABLE EXPRESSION
+        TABLE id: 3, table_name: default.t1
+      RIGHT TABLE EXPRESSION
+        TABLE id: 5, table_name: default.t2
+      JOIN EXPRESSION
+        FUNCTION id: 7, function_name: equals, function_type: ordinary, result_type: UInt8
+          ARGUMENTS
+            LIST id: 8, nodes: 2
+              COLUMN id: 9, column_name: id, result_type: UInt64, source_id: 3
+              COLUMN id: 10, column_name: id, result_type: UInt64, source_id: 5
+  ORDER BY
+    LIST id: 11, nodes: 2
+      SORT id: 12, sort_direction: ASCENDING, with_fill: 0
+        EXPRESSION
+          COLUMN id: 13, column_name: id, result_type: UInt64, source_id: 3
+      SORT id: 14, sort_direction: ASCENDING, with_fill: 0
+        EXPRESSION
+          COLUMN id: 15, column_name: id, result_type: UInt64, source_id: 5
 [0,1,2]
 [0,1,2]
 [0,1,2]
diff --git a/tests/queries/0_stateless/01323_redundant_functions_in_order_by.sql b/tests/queries/0_stateless/01323_redundant_functions_in_order_by.sql
index 3573773b76c..5cdc4164d56 100644
--- a/tests/queries/0_stateless/01323_redundant_functions_in_order_by.sql
+++ b/tests/queries/0_stateless/01323_redundant_functions_in_order_by.sql
@@ -31,6 +31,13 @@ EXPLAIN SYNTAX SELECT key, a FROM test ORDER BY key, exp(key + a);
 EXPLAIN QUERY TREE run_passes=1 SELECT key, a FROM test ORDER BY key, exp(key + a);
 EXPLAIN QUERY TREE run_passes=1 SELECT key FROM test GROUP BY key ORDER BY avg(a), key;
 
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+CREATE TABLE t1 (id UInt64) ENGINE = MergeTree() ORDER BY id;
+CREATE TABLE t2 (id UInt64) ENGINE = MergeTree() ORDER BY id;
+
+EXPLAIN QUERY TREE run_passes=1 SELECT * FROM t1 INNER JOIN t2 ON t1.id = t2.id ORDER BY t1.id, t2.id;
+
 set optimize_redundant_functions_in_order_by = 0;
 
 SELECT groupArray(x) from (SELECT number as x FROM numbers(3) ORDER BY x, exp(x));
@@ -46,4 +53,6 @@ EXPLAIN SYNTAX SELECT * FROM (SELECT number + 2 AS key FROM numbers(4)) s FULL J
 EXPLAIN SYNTAX SELECT key, a FROM test ORDER BY key, a, exp(key + a);
 EXPLAIN SYNTAX SELECT key, a FROM test ORDER BY key, exp(key + a);
 
+DROP TABLE t1;
+DROP TABLE t2;
 DROP TABLE test;