column to table matching refactoring

dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp

@@ -90,9 +90,12 @@ public:
     using TypeToVisit = const ASTFunction;
 
     CheckExpressionVisitorData(const std::vector<JoinedTable> & tables_)
-        : tables(tables_)
+        : joined_tables(tables_)
         , ands_only(true)
-    {}
+    {
+        for (auto & joined : joined_tables)
+            tables.push_back(joined.table);
+    }
 
     void visit(const ASTFunction & node, ASTPtr & ast)
     {
@@ -156,7 +159,8 @@ public:
     }
 
 private:
-    const std::vector<JoinedTable> & tables;
+    const std::vector<JoinedTable> & joined_tables;
+    std::vector<DatabaseAndTableWithAlias> tables;
     std::map<size_t, std::vector<ASTPtr>> asts_to_join_on;
     bool ands_only;
 
@@ -180,31 +184,16 @@ private:
     /// @return table position to attach expression to or 0.
     size_t checkIdentifiers(const ASTIdentifier & left, const ASTIdentifier & right)
     {
-        /// {best_match, best_table_pos}
+        size_t left_table_pos = 0;
-        std::pair<size_t, size_t> left_best{0, 0};
+        bool left_match = IdentifierSemantic::chooseTable(left, tables, left_table_pos);
-        std::pair<size_t, size_t> right_best{0, 0};
 
-        for (size_t i = 0; i < tables.size(); ++i)
+        size_t right_table_pos = 0;
+        bool right_match = IdentifierSemantic::chooseTable(right, tables, right_table_pos);
+
+        if (left_match && right_match && (left_table_pos != right_table_pos))
         {
-            size_t match = IdentifierSemantic::canReferColumnToTable(left, tables[i].table);
+            size_t table_pos = std::max(left_table_pos, right_table_pos);
-            if (match > left_best.first)
+            if (joined_tables[table_pos].canAttachOnExpression())
-            {
-                left_best.first = match;
-                left_best.second = i;
-            }
-
-            match = IdentifierSemantic::canReferColumnToTable(right, tables[i].table);
-            if (match > right_best.first)
-            {
-                right_best.first = match;
-                right_best.second = i;
-            }
-        }
-
-        if (left_best.first && right_best.first && (left_best.second != right_best.second))
-        {
-            size_t table_pos = std::max(left_best.second, right_best.second);
-            if (tables[table_pos].canAttachOnExpression())
                 return table_pos;
         }
         return 0;
@@ -212,20 +201,10 @@ private:
 
     size_t checkIdentifier(const ASTIdentifier & identifier)
     {
-        size_t best_match = 0;
         size_t best_table_pos = 0;
+        bool match = IdentifierSemantic::chooseTable(identifier, tables, best_table_pos);
 
-        for (size_t i = 0; i < tables.size(); ++i)
+        if (match && joined_tables[best_table_pos].canAttachOnExpression())
-        {
-            size_t match = IdentifierSemantic::canReferColumnToTable(identifier, tables[i].table);
-            if (match > best_match)
-            {
-                best_match = match;
-                best_table_pos = i;
-            }
-        }
-
-        if (best_match && tables[best_table_pos].canAttachOnExpression())
             return best_table_pos;
         return 0;
     }

dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp

@@ -28,17 +28,9 @@ void FindIdentifierBestTableData::visit(ASTIdentifier & identifier, ASTPtr &)
     }
     else
     {
-        // FIXME: make a better matcher using `names`?
+        size_t best_table_pos = 0;
-        size_t best_match = 0;
+        if (IdentifierSemantic::chooseTable(identifier, tables, best_table_pos))
-        for (const auto & table_names : tables)
+            best_table = &tables[best_table_pos].first;
-        {
-            if (size_t match = IdentifierSemantic::canReferColumnToTable(identifier, table_names.first))
-                if (match > best_match)
-                {
-                    best_match = match;
-                    best_table = &table_names.first;
-                }
-        }
     }
 
     identifier_table.emplace_back(&identifier, best_table);

dbms/src/Interpreters/IdentifierSemantic.cpp

@@ -5,6 +5,37 @@
 namespace DB
 {
 
+namespace
+{
+
+const DatabaseAndTableWithAlias & extractTable(const DatabaseAndTableWithAlias & table)
+{
+    return table;
+}
+
+const DatabaseAndTableWithAlias & extractTable(const TableWithColumnNames & table)
+{
+    return table.first;
+}
+
+template <typename T>
+bool tryChooseTable(const ASTIdentifier & identifier, const std::vector<T> & tables, size_t & best_table_pos)
+{
+    best_table_pos = 0;
+    size_t best_match = 0;
+    for (size_t i = 0; i < tables.size(); ++i)
+        if (size_t match = IdentifierSemantic::canReferColumnToTable(identifier, extractTable(tables[i])))
+            if (match > best_match)
+            {
+                best_match = match;
+                best_table_pos = i;
+            }
+
+    return best_match;
+}
+
+}
+
 std::optional<String> IdentifierSemantic::getColumnName(const ASTIdentifier & node)
 {
     if (!node.semantic->special)
@@ -57,26 +88,16 @@ size_t IdentifierSemantic::getMembership(const ASTIdentifier & identifier)
     return identifier.semantic->membership;
 }
 
-bool IdentifierSemantic::trySetMembership(ASTIdentifier & identifier, const std::vector<TableWithColumnNames> & tables,
+bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector<DatabaseAndTableWithAlias> & tables,
-                                          size_t & best_table_pos)
+                                     size_t & best_table_pos)
 {
-    best_table_pos = 0;
+    return tryChooseTable<DatabaseAndTableWithAlias>(identifier, tables, best_table_pos);
-    size_t best_match = 0;
+}
-    for (size_t i = 0; i < tables.size(); ++i)
-        if (size_t match = canReferColumnToTable(identifier, tables[i].first))
-            if (match > best_match)
-            {
-                best_match = match;
-                best_table_pos = i;
-            }
 
-    if (best_match)
+bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector<TableWithColumnNames> & tables,
-    {
+                                     size_t & best_table_pos)
-        setMembership(identifier, best_table_pos + 1);
+{
-        return true;
+    return tryChooseTable<TableWithColumnNames>(identifier, tables, best_table_pos);
-    }
-
-    return false;
 }
 
 std::pair<String, String> IdentifierSemantic::extractDatabaseAndTable(const ASTIdentifier & identifier)

dbms/src/Interpreters/IdentifierSemantic.h

@@ -35,7 +35,8 @@ struct IdentifierSemantic
     static bool canBeAlias(const ASTIdentifier & identifier);
     static void setMembership(ASTIdentifier & identifier, size_t table_no);
     static size_t getMembership(const ASTIdentifier & identifier);
-    static bool trySetMembership(ASTIdentifier & identifier, const std::vector<TableWithColumnNames> & tables, size_t & best_table_pos);
+    static bool chooseTable(const ASTIdentifier &, const std::vector<DatabaseAndTableWithAlias> & tables, size_t & best_table_pos);
+    static bool chooseTable(const ASTIdentifier &, const std::vector<TableWithColumnNames> & tables, size_t & best_table_pos);
 
 private:
     static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & database, const String & table);

dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp

@@ -163,7 +163,13 @@ struct ColumnAliasesMatcher
                 auto it = rev_aliases.find(long_name);
                 if (it == rev_aliases.end())
                 {
-                    bool last_table = IdentifierSemantic::canReferColumnToTable(*identifier, tables.back());
+                    bool last_table = false;
+                    {
+                        size_t best_table_pos = 0;
+                        if (IdentifierSemantic::chooseTable(*identifier, tables, best_table_pos))
+                            last_table = (best_table_pos + 1 == tables.size());
+                    }
+
                     if (!last_table)
                     {
                         String alias = hide_prefix + long_name;

dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp

@@ -62,7 +62,8 @@ void TranslateQualifiedNamesMatcher::visit(ASTIdentifier & identifier, ASTPtr &,
     if (IdentifierSemantic::getColumnName(identifier))
     {
         size_t best_table_pos = 0;
-        IdentifierSemantic::trySetMembership(identifier, data.tables, best_table_pos);
+        if (IdentifierSemantic::chooseTable(identifier, data.tables, best_table_pos))
+            IdentifierSemantic::setMembership(identifier, best_table_pos + 1);
 
         /// In case if column from the joined table are in source columns, change it's name to qualified.
         if (best_table_pos && data.hasColumn(identifier.shortName()))