Merge pull request ClickHouse#63519 from ClickHouse/analyzer-fix-join-projection-name

Analyzer: Fix column projection name after column type promotion in join

Author: KochetovNicolai

src/Analyzer/Resolve/IdentifierResolveScope.h

@@ -147,6 +147,8 @@ struct IdentifierResolveScope
     /// Table column name to column node. Valid only during table ALIAS columns resolve.
     ColumnNameToColumnNodeMap column_name_to_column_node;
 
+    std::list<std::unordered_map<std::string, ColumnNodePtr> *> join_using_columns;
+
     /// CTE name to query node
     std::unordered_map<std::string, QueryTreeNodePtr> cte_name_to_query_node;
 

src/Analyzer/Resolve/IdentifierResolver.cpp

@@ -30,6 +30,7 @@
 #include <Analyzer/Resolve/TypoCorrection.h>
 
 #include <Core/Settings.h>
+#include <iostream>
 
 namespace DB
 {
@@ -329,6 +330,21 @@ bool IdentifierResolver::tryBindIdentifierToAliases(const IdentifierLookup & ide
     return scope.aliases.find(identifier_lookup, ScopeAliases::FindOption::FIRST_NAME) != nullptr;
 }
 
+bool IdentifierResolver::tryBindIdentifierToJoinUsingColumn(const IdentifierLookup & identifier_lookup, const IdentifierResolveScope & scope)
+{
+    for (const auto * join_using : scope.join_using_columns)
+    {
+        for (const auto & [using_column_name, _] : *join_using)
+        {
+            // std::cerr << identifier_lookup.identifier.getFullName() << " <===========> " << using_column_name << std::endl;
+            if (identifier_lookup.identifier.getFullName() == using_column_name)
+                return true;
+        }
+    }
+
+    return false;
+}
+
 /** Resolve identifier from table columns.
   *
   * 1. If table column nodes are empty or identifier is not expression lookup return nullptr.
@@ -634,11 +650,15 @@ IdentifierResolveResult IdentifierResolver::tryResolveIdentifierFromStorage(
             tryBindIdentifierToTableExpressions(column_identifier_lookup, table_expression_node, scope))
             break;
 
+        if (tryBindIdentifierToJoinUsingColumn(column_identifier_lookup, scope))
+            break;
+
         qualified_identifier = std::move(qualified_identifier_with_removed_part);
     }
 
     auto qualified_identifier_full_name = qualified_identifier.getFullName();
     node_to_projection_name.emplace(result_expression, std::move(qualified_identifier_full_name));
+    // std::cerr << "resolved from storage : " << qualified_identifier.getFullName() << " as " << result_expression->dumpTree() << std::endl;
 
     return { .resolved_identifier = result_expression, .resolve_place = IdentifierResolvePlace::JOIN_TREE };
 }
@@ -857,8 +877,36 @@ IdentifierResolveResult IdentifierResolver::tryResolveIdentifierFromJoin(const I
     IdentifierResolveScope & scope)
 {
     const auto & from_join_node = table_expression_node->as<const JoinNode &>();
-    auto left_resolved_identifier = tryResolveIdentifierFromJoinTreeNode(identifier_lookup, from_join_node.getLeftTableExpression(), scope).resolved_identifier;
-    auto right_resolved_identifier = tryResolveIdentifierFromJoinTreeNode(identifier_lookup, from_join_node.getRightTableExpression(), scope).resolved_identifier;
+    JoinKind join_kind = from_join_node.getKind();
+
+    bool join_node_in_resolve_process = scope.table_expressions_in_resolve_process.contains(table_expression_node.get());
+    std::unordered_map<std::string, ColumnNodePtr> join_using_column_name_to_column_node;
+
+    if (!join_node_in_resolve_process && from_join_node.isUsingJoinExpression())
+    {
+        auto & join_using_list = from_join_node.getJoinExpression()->as<ListNode &>();
+        for (auto & join_using_node : join_using_list.getNodes())
+        {
+            auto & column_node = join_using_node->as<ColumnNode &>();
+            join_using_column_name_to_column_node.emplace(column_node.getColumnName(), std::static_pointer_cast<ColumnNode>(join_using_node));
+        }
+    }
+
+    auto try_resolve_identifier_from_join_tree_node = [&](const QueryTreeNodePtr & join_tree_node, bool may_be_override_by_using_column)
+    {
+        if (may_be_override_by_using_column && !join_using_column_name_to_column_node.empty())
+            scope.join_using_columns.push_back(&join_using_column_name_to_column_node);
+
+        auto res = tryResolveIdentifierFromJoinTreeNode(identifier_lookup, join_tree_node, scope);
+
+        if (may_be_override_by_using_column && !join_using_column_name_to_column_node.empty())
+            scope.join_using_columns.pop_back();
+
+        return std::move(res.resolved_identifier);
+    };
+
+    auto left_resolved_identifier = try_resolve_identifier_from_join_tree_node(from_join_node.getLeftTableExpression(), join_kind == JoinKind::Right);
+    auto right_resolved_identifier = try_resolve_identifier_from_join_tree_node(from_join_node.getRightTableExpression(), join_kind != JoinKind::Right);
 
     if (!identifier_lookup.isExpressionLookup())
     {
@@ -875,20 +923,6 @@ IdentifierResolveResult IdentifierResolver::tryResolveIdentifierFromJoin(const I
             };
     }
 
-    bool join_node_in_resolve_process = scope.table_expressions_in_resolve_process.contains(table_expression_node.get());
-
-    std::unordered_map<std::string, ColumnNodePtr> join_using_column_name_to_column_node;
-
-    if (!join_node_in_resolve_process && from_join_node.isUsingJoinExpression())
-    {
-        auto & join_using_list = from_join_node.getJoinExpression()->as<ListNode &>();
-        for (auto & join_using_node : join_using_list.getNodes())
-        {
-            auto & column_node = join_using_node->as<ColumnNode &>();
-            join_using_column_name_to_column_node.emplace(column_node.getColumnName(), std::static_pointer_cast<ColumnNode>(join_using_node));
-        }
-    }
-
     auto check_nested_column_not_in_using = [&join_using_column_name_to_column_node, &identifier_lookup](const QueryTreeNodePtr & node)
     {
         /** tldr: When an identifier is resolved into the function `nested` or `getSubcolumn`, and
@@ -950,13 +984,45 @@ IdentifierResolveResult IdentifierResolver::tryResolveIdentifierFromJoin(const I
     std::optional<JoinTableSide> resolved_side;
     QueryTreeNodePtr resolved_identifier;
 
-    JoinKind join_kind = from_join_node.getKind();
+    auto convert_resolved_result_type_if_needed = [](
+        const QueryTreeNodePtr & resolved_identifier_candidate,
+        const std::unordered_map<std::string, ColumnNodePtr> & using_column_name_to_column_node,
+        QueryTreeNodePtr & resolve_result,
+        IdentifierResolveScope & current_scope,
+        std::unordered_map<QueryTreeNodePtr, ProjectionName> & projection_name_mapping)
+    {
+        auto & resolved_column = resolved_identifier_candidate->as<ColumnNode &>();
+        auto using_column_node_it = using_column_name_to_column_node.find(resolved_column.getColumnName());
+        if (using_column_node_it != using_column_name_to_column_node.end() &&
+            !using_column_node_it->second->getColumnType()->equals(*resolved_column.getColumnType()))
+        {
+            // std::cerr << "... fixing type for " << resolved_column.dumpTree() << std::endl;
+            auto resolved_column_clone = std::static_pointer_cast<ColumnNode>(resolved_column.clone());
+            resolved_column_clone->setColumnType(using_column_node_it->second->getColumnType());
+
+            auto projection_name_it = projection_name_mapping.find(resolved_identifier_candidate);
+            if (projection_name_it != projection_name_mapping.end())
+            {
+                projection_name_mapping[resolved_column_clone] = projection_name_it->second;
+                // std::cerr << ".. upd name " << projection_name_it->second << " for col " << resolved_column_clone->dumpTree() << std::endl;
+            }
+
+            resolve_result = std::move(resolved_column_clone);
+
+            if (!resolve_result->isEqual(*using_column_node_it->second))
+                current_scope.join_columns_with_changed_types[resolve_result] = using_column_node_it->second;
+        }
+    };
 
     /// If columns from left or right table were missed Object(Nullable('json')) subcolumns, they will be replaced
     /// to ConstantNode(NULL), which can't be cast to ColumnNode, so we resolve it here.
     if (auto missed_subcolumn_identifier = checkIsMissedObjectJSONSubcolumn(left_resolved_identifier, right_resolved_identifier))
         return { .resolved_identifier = missed_subcolumn_identifier, .resolve_place = IdentifierResolvePlace::JOIN_TREE };
 
+
+    // for (const auto & [k, v] : join_using_column_name_to_column_node)
+    //     std::cerr << k << " -> " << v->dumpTree() << std::endl;
+
     if (left_resolved_identifier && right_resolved_identifier)
     {
         auto using_column_node_it = join_using_column_name_to_column_node.end();
@@ -1036,18 +1102,7 @@ IdentifierResolveResult IdentifierResolver::tryResolveIdentifierFromJoin(const I
         }
         else
         {
-            auto & left_resolved_column = left_resolved_identifier->as<ColumnNode &>();
-            auto using_column_node_it = join_using_column_name_to_column_node.find(left_resolved_column.getColumnName());
-            if (using_column_node_it != join_using_column_name_to_column_node.end() &&
-                !using_column_node_it->second->getColumnType()->equals(*left_resolved_column.getColumnType()))
-            {
-                auto left_resolved_column_clone = std::static_pointer_cast<ColumnNode>(left_resolved_column.clone());
-                left_resolved_column_clone->setColumnType(using_column_node_it->second->getColumnType());
-                resolved_identifier = std::move(left_resolved_column_clone);
-
-                if (!resolved_identifier->isEqual(*using_column_node_it->second))
-                    scope.join_columns_with_changed_types[resolved_identifier] = using_column_node_it->second;
-            }
+            convert_resolved_result_type_if_needed(left_resolved_identifier, join_using_column_name_to_column_node, resolved_identifier, scope, node_to_projection_name);
         }
     }
     else if (right_resolved_identifier)
@@ -1061,17 +1116,7 @@ IdentifierResolveResult IdentifierResolver::tryResolveIdentifierFromJoin(const I
         }
         else
         {
-            auto & right_resolved_column = right_resolved_identifier->as<ColumnNode &>();
-            auto using_column_node_it = join_using_column_name_to_column_node.find(right_resolved_column.getColumnName());
-            if (using_column_node_it != join_using_column_name_to_column_node.end() &&
-                !using_column_node_it->second->getColumnType()->equals(*right_resolved_column.getColumnType()))
-            {
-                auto right_resolved_column_clone = std::static_pointer_cast<ColumnNode>(right_resolved_column.clone());
-                right_resolved_column_clone->setColumnType(using_column_node_it->second->getColumnType());
-                resolved_identifier = std::move(right_resolved_column_clone);
-                if (!resolved_identifier->isEqual(*using_column_node_it->second))
-                    scope.join_columns_with_changed_types[resolved_identifier] = using_column_node_it->second;
-            }
+            convert_resolved_result_type_if_needed(right_resolved_identifier, join_using_column_name_to_column_node, resolved_identifier, scope, node_to_projection_name);
         }
     }
 
@@ -1087,10 +1132,12 @@ IdentifierResolveResult IdentifierResolver::tryResolveIdentifierFromJoin(const I
         auto nullable_resolved_identifier = convertJoinedColumnTypeToNullIfNeeded(resolved_identifier, join_kind, resolved_side, scope);
         if (nullable_resolved_identifier)
         {
+            // std::cerr << ".. convert to null " << nullable_resolved_identifier->dumpTree() << std::endl;
             resolved_identifier = nullable_resolved_identifier;
             /// Set the same projection name for new nullable node
             if (projection_name_it != node_to_projection_name.end())
             {
+                // std::cerr << "... upd name for null " << projection_name_it->second << " -> " << resolved_identifier->dumpTree() << std::endl;
                 node_to_projection_name.emplace(resolved_identifier, projection_name_it->second);
             }
         }

src/Analyzer/Resolve/IdentifierResolver.h

@@ -48,6 +48,10 @@ class IdentifierResolver
         const IdentifierLookup & identifier_lookup,
         const IdentifierResolveScope & scope);
 
+    static bool tryBindIdentifierToJoinUsingColumn(
+        const IdentifierLookup & identifier_lookup,
+        const IdentifierResolveScope & scope);
+
     static bool tryBindIdentifierToTableExpression(
         const IdentifierLookup & identifier_lookup,
         const QueryTreeNodePtr & table_expression_node,

tests/queries/0_stateless/03149_analyzer_join_projection_name.reference

@@ -0,0 +1,19 @@
+analyzer=1, join with dictionary
+   ┌──uid─┬─name─┬─gid─┬─gname──┐
+1. │ 1231 │ John │ 1   │ Group1 │
+   └──────┴──────┴─────┴────────┘
+
+analyzer=1, join with table
+   ┌──uid─┬─name─┬─gid─┬─gname──┐
+1. │ 1231 │ John │ 1   │ Group1 │
+   └──────┴──────┴─────┴────────┘
+
+analyzer=0, join with dictionary
+   ┌──uid─┬─name─┬─gid─┬─gname──┐
+1. │ 1231 │ John │ 1   │ Group1 │
+   └──────┴──────┴─────┴────────┘
+
+analyzer=0, join with table
+   ┌──uid─┬─name─┬─gid─┬─gname──┐
+1. │ 1231 │ John │ 1   │ Group1 │
+   └──────┴──────┴─────┴────────┘

tests/queries/0_stateless/03149_analyzer_join_projection_name.sql

@@ -0,0 +1,51 @@
+DROP DATABASE IF EXISTS db_for_dict_03149;
+CREATE DATABASE db_for_dict_03149;
+
+CREATE TABLE db_for_dict_03149.users (uid Int16, name String, gid LowCardinality(String), gname LowCardinality(String))
+  ENGINE=MergeTree order by tuple();
+CREATE TABLE db_for_dict_03149.groups (gid LowCardinality(String), gname LowCardinality(String))
+  ENGINE=MergeTree order by tuple();
+
+CREATE DICTIONARY db_for_dict_03149.groups_dict (
+    gid String, gname String
+)
+PRIMARY KEY gid, gname
+LAYOUT(COMPLEX_KEY_HASHED())
+SOURCE(CLICKHOUSE(HOST 'localhost' PORT tcpPort() QUERY 'select * from db_for_dict_03149.groups'))
+LIFETIME(MIN 0 MAX 0);
+
+
+INSERT INTO db_for_dict_03149.groups VALUES ('1', 'Group1');
+
+INSERT INTO db_for_dict_03149.users VALUES (1231, 'John', '1', 'Group1');
+
+select 'analyzer=1, join with dictionary';
+
+SELECT u.uid, u.name, u.gid, u.gname
+FROM db_for_dict_03149.users u left join db_for_dict_03149.groups_dict g using gid, gname
+format PrettyCompactMonoBlock;
+
+select '';
+select 'analyzer=1, join with table';
+
+SELECT u.uid, u.name, u.gid, u.gname
+FROM db_for_dict_03149.users u left join db_for_dict_03149.groups g using gid, gname
+format PrettyCompactMonoBlock;
+
+
+set allow_experimental_analyzer=0;
+
+select '';
+select 'analyzer=0, join with dictionary';
+
+SELECT u.uid, u.name, u.gid, u.gname
+FROM db_for_dict_03149.users u left join db_for_dict_03149.groups_dict g using gid, gname
+format PrettyCompactMonoBlock;
+
+select '';
+select 'analyzer=0, join with table';
+SELECT u.uid, u.name, u.gid, u.gname
+FROM db_for_dict_03149.users u left join db_for_dict_03149.groups g using gid, gname
+format PrettyCompactMonoBlock;
+
+DROP DATABASE IF EXISTS db_for_dict_03149;

tests/queries/0_stateless/03149_analyzer_join_projection_name_2.reference

@@ -0,0 +1,6 @@
+   ┌──uid─┬─name───┬─gid─┬─gname──┐
+1. │ 1231 │ John   │ 1   │ Group1 │
+2. │ 1234 │ Test   │ 2   │ Group1 │
+3. │ 6666 │ Ksenia │ 1   │ Group1 │
+4. │ 8888 │ Alice  │ 1   │ Group1 │
+   └──────┴────────┴─────┴────────┘

tests/queries/0_stateless/03149_analyzer_join_projection_name_2.sql

@@ -0,0 +1,33 @@
+DROP DATABASE IF EXISTS db_for_dict_03149;
+CREATE DATABASE db_for_dict_03149;
+
+CREATE TABLE db_for_dict_03149.users (uid Int16, name String, gid LowCardinality(String), gname LowCardinality(String))
+  ENGINE=MergeTree order by tuple();
+CREATE TABLE db_for_dict_03149.groups (gid LowCardinality(String), gname LowCardinality(String))
+  ENGINE=MergeTree order by tuple();
+
+CREATE TABLE db_for_dict_03149.target (uid Int16, name String, gid LowCardinality(String), gname LowCardinality(String))
+  ENGINE=MergeTree order by tuple();
+
+CREATE DICTIONARY db_for_dict_03149.groups_dict (
+    gid String, gname String
+)
+PRIMARY KEY gid, gname
+LAYOUT(COMPLEX_KEY_HASHED())
+SOURCE(CLICKHOUSE(QUERY 'select * from db_for_dict_03149.groups'))
+LIFETIME(MIN 0 MAX 0);
+
+CREATE MATERIALIZED VIEW db_for_dict_03149.mv to db_for_dict_03149.target AS
+SELECT u.uid, u.name, u.gid, u.gname
+FROM db_for_dict_03149.users u left join db_for_dict_03149.groups_dict g using gid, gname;
+
+INSERT INTO db_for_dict_03149.groups VALUES ('1', 'Group1');
+
+INSERT INTO db_for_dict_03149.users VALUES (1231, 'John', '1', 'Group1');
+INSERT INTO db_for_dict_03149.users VALUES (6666, 'Ksenia', '1', 'Group1');
+INSERT INTO db_for_dict_03149.users VALUES (8888, 'Alice', '1', 'Group1');
+INSERT INTO db_for_dict_03149.users VALUES (1234, 'Test', '2', 'Group1');
+
+SELECT * FROM db_for_dict_03149.target ORDER BY uid format PrettyCompactMonoBlock;
+
+DROP DATABASE IF EXISTS db_for_dict_03149;

tests/queries/0_stateless/03408_cte_self_reference.sql

@@ -1,3 +1,4 @@
+-- Tags: no-tsan
 SET enable_analyzer = 1;
 
 WITH `cte1` AS (