diff --git a/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp b/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp index 52cb05d35ea..5ac28bc6dde 100644 --- a/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp +++ b/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp @@ -90,9 +90,12 @@ public: using TypeToVisit = const ASTFunction; CheckExpressionVisitorData(const std::vector & 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 & tables; + const std::vector & joined_tables; + std::vector tables; std::map> 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} - std::pair left_best{0, 0}; - std::pair right_best{0, 0}; + size_t left_table_pos = 0; + bool left_match = IdentifierSemantic::chooseTable(left, tables, left_table_pos); - 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); - if (match > left_best.first) - { - 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()) + size_t table_pos = std::max(left_table_pos, right_table_pos); + if (joined_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) - { - 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()) + if (match && joined_tables[best_table_pos].canAttachOnExpression()) return best_table_pos; return 0; } diff --git a/dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp b/dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp index 8173ce3256a..daf9ca57fb9 100644 --- a/dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp +++ b/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_match = 0; - for (const auto & table_names : tables) - { - if (size_t match = IdentifierSemantic::canReferColumnToTable(identifier, table_names.first)) - if (match > best_match) - { - best_match = match; - best_table = &table_names.first; - } - } + size_t best_table_pos = 0; + if (IdentifierSemantic::chooseTable(identifier, tables, best_table_pos)) + best_table = &tables[best_table_pos].first; } identifier_table.emplace_back(&identifier, best_table); diff --git a/dbms/src/Interpreters/IdentifierSemantic.cpp b/dbms/src/Interpreters/IdentifierSemantic.cpp index 6b74dc2d8d1..f5006159aa9 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.cpp +++ b/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 +bool tryChooseTable(const ASTIdentifier & identifier, const std::vector & 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 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 & tables, - size_t & best_table_pos) +bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & 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 = canReferColumnToTable(identifier, tables[i].first)) - if (match > best_match) - { - best_match = match; - best_table_pos = i; - } + return tryChooseTable(identifier, tables, best_table_pos); +} - if (best_match) - { - setMembership(identifier, best_table_pos + 1); - return true; - } - - return false; +bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & tables, + size_t & best_table_pos) +{ + return tryChooseTable(identifier, tables, best_table_pos); } std::pair IdentifierSemantic::extractDatabaseAndTable(const ASTIdentifier & identifier) diff --git a/dbms/src/Interpreters/IdentifierSemantic.h b/dbms/src/Interpreters/IdentifierSemantic.h index 4fde404488c..7403f5d2340 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.h +++ b/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 & tables, size_t & best_table_pos); + static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos); + static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos); private: static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & database, const String & table); diff --git a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp b/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp index b60e6533921..f807bfb7acb 100644 --- a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp +++ b/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; diff --git a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp index e05387b5aa0..7226ce9d4dd 100644 --- a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp +++ b/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()))