QueryAliasesMatcher via InDepthNodeVisitor (bottom to top) CLICKHOUSE-3996

dbms/src/Interpreters/InDepthNodeVisitor.h

@@ -38,7 +38,7 @@ public:
     }
 
 private:
-    MatcherData & data;
+    Data & data;
     size_t visit_depth;
     std::ostream * ostr;
 

dbms/src/Interpreters/PredicateExpressionsOptimizer.cpp

@@ -313,8 +313,8 @@ ASTs PredicateExpressionsOptimizer::getSelectQueryProjectionColumns(ASTPtr & ast
 
     TranslateQualifiedNamesMatcher::Data qn_visitor_data{{}, tables};
     TranslateQualifiedNamesVisitor(qn_visitor_data).visit(ast);
-    QueryAliasesVisitor query_aliases_visitor(aliases);
-    query_aliases_visitor.visit(ast);
+    QueryAliasesMatcher::Data query_aliases_data{aliases};
+    QueryAliasesVisitor(query_aliases_data).visit(ast);
     QueryNormalizer(ast, aliases, settings, {}, {}).perform();
 
     for (const auto & projection_column : select_query->select_expression_list->children)

dbms/src/Interpreters/QueryAliasesVisitor.cpp

@@ -1,5 +1,6 @@
 #include <ostream>
 #include <sstream>
+
 #include <Common/typeid_cast.h>
 #include <Interpreters/QueryAliasesVisitor.h>
 #include <Parsers/ASTTablesInSelectQuery.h>
@@ -16,33 +17,62 @@ namespace ErrorCodes
     extern const int MULTIPLE_EXPRESSIONS_FOR_ALIAS;
 }
 
-void QueryAliasesVisitor::visit(const ASTPtr & ast) const
+static String wrongAliasMessage(const ASTPtr & ast, const ASTPtr & prev_ast, const String & alias)
 {
-    /// Bottom-up traversal. We do not go into subqueries.
-    visitChildren(ast);
-
-    if (!tryVisit<ASTSubquery>(ast))
-    {
-        DumpASTNode dump(*ast, ostr, visit_depth, "getQueryAliases");
-        visitOther(ast);
+    std::stringstream message;
+    message << "Different expressions with the same alias " << backQuoteIfNeed(alias) << ":" << std::endl;
+    formatAST(*ast, message, false, true);
+    message << std::endl << "and" << std::endl;
+    formatAST(*prev_ast, message, false, true);
+    message << std::endl;
+    return message.str();
 }
+
+
+bool QueryAliasesMatcher::needChildVisit(ASTPtr & node, const ASTPtr &)
+{
+    /// Don't descent into table functions and subqueries and special case for ArrayJoin.
+    if (typeid_cast<ASTTableExpression *>(node.get()) ||
+        typeid_cast<ASTSelectWithUnionQuery *>(node.get()) ||
+        typeid_cast<ASTArrayJoin *>(node.get()))
+        return false;
+    return true;
+}
+
+std::vector<ASTPtr> QueryAliasesMatcher::visit(ASTPtr & ast, Data & data)
+{
+    if (auto * t = typeid_cast<ASTSubquery *>(ast.get()))
+        return visit(*t, ast, data);
+    if (auto * t = typeid_cast<ASTArrayJoin *>(ast.get()))
+        return visit(*t, ast, data);
+
+    visitOther(ast, data);
+    return {};
 }
 
 /// The top-level aliases in the ARRAY JOIN section have a special meaning, we will not add them
 /// (skip the expression list itself and its children).
-void QueryAliasesVisitor::visit(const ASTArrayJoin &, const ASTPtr & ast) const
+std::vector<ASTPtr> QueryAliasesMatcher::visit(const ASTArrayJoin &, const ASTPtr & ast, Data & data)
 {
+    visitOther(ast, data);
+
+    /// @warning It breaks botom-to-top order (childs processed after node here), could lead to some effects.
+    /// It's possible to add ast back to result vec to save order. It will need two phase ASTArrayJoin visit (setting phase in data).
+    std::vector<ASTPtr> out;
     for (auto & child1 : ast->children)
         for (auto & child2 : child1->children)
             for (auto & child3 : child2->children)
-                visit(child3);
+                out.push_back(child3);
+    return out;
 }
 
 /// set unique aliases for all subqueries. this is needed, because:
 /// 1) content of subqueries could change after recursive analysis, and auto-generated column names could become incorrect
 /// 2) result of different scalar subqueries can be cached inside expressions compilation cache and must have different names
-void QueryAliasesVisitor::visit(ASTSubquery & subquery, const ASTPtr & ast) const
+std::vector<ASTPtr> QueryAliasesMatcher::visit(ASTSubquery & subquery, const ASTPtr & ast, Data & data)
 {
+    Aliases & aliases = data.aliases;
+
     static std::atomic_uint64_t subquery_index = 0;
 
     if (subquery.alias.empty())
@@ -59,42 +89,22 @@ void QueryAliasesVisitor::visit(ASTSubquery & subquery, const ASTPtr & ast) cons
         aliases[alias] = ast;
     }
     else
-        visitOther(ast);
+        visitOther(ast, data);
+    return {};
 }
 
-void QueryAliasesVisitor::visitOther(const ASTPtr & ast) const
+void QueryAliasesMatcher::visitOther(const ASTPtr & ast, Data & data)
 {
+    Aliases & aliases = data.aliases;
+
     String alias = ast->tryGetAlias();
     if (!alias.empty())
     {
         if (aliases.count(alias) && ast->getTreeHash() != aliases[alias]->getTreeHash())
-            throw Exception(wrongAliasMessage(ast, alias), ErrorCodes::MULTIPLE_EXPRESSIONS_FOR_ALIAS);
+            throw Exception(wrongAliasMessage(ast, aliases[alias], alias), ErrorCodes::MULTIPLE_EXPRESSIONS_FOR_ALIAS);
 
         aliases[alias] = ast;
     }
 }
 
-void QueryAliasesVisitor::visitChildren(const ASTPtr & ast) const
-{
-    for (auto & child : ast->children)
-    {
-        /// Don't descent into table functions and subqueries and special case for ArrayJoin.
-        if (!tryVisit<ASTTableExpression>(ast) &&
-            !tryVisit<ASTSelectWithUnionQuery>(ast) &&
-            !tryVisit<ASTArrayJoin>(ast))
-            visit(child);
-    }
-}
-
-String QueryAliasesVisitor::wrongAliasMessage(const ASTPtr & ast, const String & alias) const
-{
-    std::stringstream message;
-    message << "Different expressions with the same alias " << backQuoteIfNeed(alias) << ":" << std::endl;
-    formatAST(*ast, message, false, true);
-    message << std::endl << "and" << std::endl;
-    formatAST(*aliases[alias], message, false, true);
-    message << std::endl;
-    return message.str();
-}
-
 }

dbms/src/Interpreters/QueryAliasesVisitor.h

@@ -1,8 +1,7 @@
 #pragma once
 
-#include <Common/typeid_cast.h>
-#include <Parsers/DumpASTNode.h>
 #include <unordered_map>
+#include <Interpreters/InDepthNodeVisitor.h>
 
 namespace DB
 {
@@ -14,47 +13,27 @@ struct ASTArrayJoin;
 
 using Aliases = std::unordered_map<String, ASTPtr>;
 
-/// Visitors consist of functions with unified interface 'void visit(Casted & x, ASTPtr & y)', there x is y, successfully casted to Casted.
-/// Both types and fuction could have const specifiers. The second argument is used by visitor to replaces AST node (y) if needed.
-
-/// Visits AST nodes and collect their aliases in one map (with links to source nodes).
-class QueryAliasesVisitor
+/// Visits AST node to collect aliases.
+class QueryAliasesMatcher
 {
 public:
-    QueryAliasesVisitor(Aliases & aliases_, std::ostream * ostr_ = nullptr)
-    :   aliases(aliases_),
-        visit_depth(0),
-        ostr(ostr_)
-    {}
-
-    void visit(const ASTPtr & ast) const;
-
-private:
+    struct Data
+    {
         Aliases & aliases;
-    mutable size_t visit_depth;
-    std::ostream * ostr;
-
-    void visit(const ASTTableExpression &, const ASTPtr &) const {}
-    void visit(const ASTSelectWithUnionQuery &, const ASTPtr &) const {}
-
-    void visit(ASTSubquery & subquery, const ASTPtr & ast) const;
-    void visit(const ASTArrayJoin &, const ASTPtr & ast) const;
-    void visitOther(const ASTPtr & ast) const;
-    void visitChildren(const ASTPtr & ast) const;
-
-    template <typename T>
-    bool tryVisit(const ASTPtr & ast) const
-    {
-        if (T * t = typeid_cast<T *>(ast.get()))
-        {
-            DumpASTNode dump(*ast, ostr, visit_depth, "getQueryAliases");
-            visit(*t, ast);
-            return true;
-        }
-        return false;
-    }
-
-    String wrongAliasMessage(const ASTPtr & ast, const String & alias) const;
     };
 
+    static constexpr const char * label = __FILE__;
+
+    static std::vector<ASTPtr> visit(ASTPtr & ast, Data & data);
+    static bool needChildVisit(ASTPtr & node, const ASTPtr & child);
+
+private:
+    static std::vector<ASTPtr> visit(ASTSubquery & subquery, const ASTPtr & ast, Data & data);
+    static std::vector<ASTPtr> visit(const ASTArrayJoin &, const ASTPtr & ast, Data & data);
+    static void visitOther(const ASTPtr & ast, Data & data);
+};
+
+/// Visits AST nodes and collect their aliases in one map (with links to source nodes).
+using QueryAliasesVisitor = InDepthNodeVisitor<QueryAliasesMatcher, false>;
+
 }

dbms/src/Interpreters/SyntaxAnalyzer.cpp

@@ -134,8 +134,8 @@ SyntaxAnalyzerResultPtr SyntaxAnalyzer::analyze(
     /// Creates a dictionary `aliases`: alias -> ASTPtr
     {
         LogAST log;
-        QueryAliasesVisitor query_aliases_visitor(result.aliases, log.stream());
-        query_aliases_visitor.visit(query);
+        QueryAliasesMatcher::Data query_aliases_data{result.aliases};
+        QueryAliasesVisitor(query_aliases_data, log.stream()).visit(query);
     }
 
     /// Common subexpression elimination. Rewrite rules.