Merge pull request ClickHouse#52145 from ClickHouse/fix-order-by-tupl…

…e-of-window-functions

Fix ORDER BY tuple of WINDOW functions

src/Interpreters/ExpressionAnalyzer.cpp

@@ -1378,10 +1378,9 @@ void SelectQueryExpressionAnalyzer::appendWindowFunctionsArguments(
 void SelectQueryExpressionAnalyzer::appendExpressionsAfterWindowFunctions(ExpressionActionsChain & chain, bool /* only_types */)
 {
     ExpressionActionsChain::Step & step = chain.lastStep(columns_after_window);
+
     for (const auto & expression : syntax->expressions_with_window_function)
-    {
         getRootActionsForWindowFunctions(expression->clone(), true, step.actions());
-    }
 }
 
 void SelectQueryExpressionAnalyzer::appendGroupByModifiers(ActionsDAGPtr & before_aggregation, ExpressionActionsChain & chain, bool /* only_types */)
@@ -1760,9 +1759,9 @@ ExpressionAnalysisResult::ExpressionAnalysisResult(
     /// second_stage: Do I need to execute the second part of the pipeline - running on the initiating server during distributed processing.
 
     /** First we compose a chain of actions and remember the necessary steps from it.
-        *  Regardless of from_stage and to_stage, we will compose a complete sequence of actions to perform optimization and
-        *  throw out unnecessary columns based on the entire query. In unnecessary parts of the query, we will not execute subqueries.
-        */
+      * Regardless of from_stage and to_stage, we will compose a complete sequence of actions to perform optimization and
+      * throw out unnecessary columns based on the entire query. In unnecessary parts of the query, we will not execute subqueries.
+      */
 
     const ASTSelectQuery & query = *query_analyzer.getSelectQuery();
     auto context = query_analyzer.getContext();
@@ -1805,7 +1804,7 @@ ExpressionAnalysisResult::ExpressionAnalysisResult(
 
         if (storage && (query.sampleSize() || settings.parallel_replicas_count > 1))
         {
-            // we evaluate sampling for Merge lazily so we need to get all the columns
+            // we evaluate sampling for Merge lazily, so we need to get all the columns
             if (storage->getName() == "Merge")
             {
                 const auto columns = metadata_snapshot->getColumns().getAll();

src/Interpreters/GetAggregatesVisitor.cpp

@@ -1,4 +1,6 @@
 #include <Interpreters/GetAggregatesVisitor.h>
+#include <Common/checkStackSize.h>
+
 
 namespace DB
 {
@@ -13,7 +15,7 @@ struct WindowExpressionsCollectorChildInfo
     bool window_function_in_subtree = false;
 };
 
-// This visitor travers AST and collects the list of expressions which depend on
+// This visitor traverses the AST and collects the list of expressions which depend on
 // evaluation of window functions. Expression is collected only if
 // it's not a part of another expression.
 //
@@ -26,15 +28,18 @@ struct WindowExpressionsCollectorMatcher
     {
         if (child->as<ASTSubquery>() || child->as<ASTSelectQuery>())
             return false;
+
         if (auto * select = node->as<ASTSelectQuery>())
         {
-            // We don't analysis WITH statement because it might contain useless aggregates
+            // We don't analyse the WITH statement because it might contain useless aggregates
             if (child == select->with())
                 return false;
         }
-        // We procces every expression manually
+
+        // We process every expression manually
         if (auto * func = node->as<ASTFunction>())
             return false;
+
         return true;
     }
 
@@ -50,6 +55,8 @@ struct WindowExpressionsCollectorMatcher
         ASTPtr & ast,
         const ASTPtr & parent)
     {
+        checkStackSize();
+
         if (auto * func = ast->as<ASTFunction>())
         {
             if (func->is_window_function)
@@ -67,23 +74,24 @@ struct WindowExpressionsCollectorMatcher
             {
                 func->compute_after_window_functions = true;
                 if ((!parent || !parent->as<ASTFunction>()))
-                    expressions_with_window_functions.push_back(func);
+                    expressions_with_window_functions.push_back(ast);
             }
 
             return result;
         }
         return {};
     }
 
-    std::vector<const ASTFunction *> expressions_with_window_functions {};
+    ASTs expressions_with_window_functions;
 };
 
 using WindowExpressionsCollectorVisitor = InDepthNodeVisitorWithChildInfo<WindowExpressionsCollectorMatcher>;
 
-std::vector<const ASTFunction *> getExpressionsWithWindowFunctions(ASTPtr & ast)
+ASTs getExpressionsWithWindowFunctions(ASTPtr & ast)
 {
     WindowExpressionsCollectorVisitor visitor;
     visitor.visit(ast);
+
     return std::move(visitor.expressions_with_window_functions);
 }
 

src/Interpreters/GetAggregatesVisitor.h

@@ -114,6 +114,6 @@ inline void assertNoAggregates(const ASTPtr & ast, const char * description)
     GetAggregatesVisitor(data).visit(ast);
 }
 
-std::vector<const ASTFunction *> getExpressionsWithWindowFunctions(ASTPtr & ast);
+ASTs getExpressionsWithWindowFunctions(ASTPtr & ast);
 
 }

src/Interpreters/TreeRewriter.cpp

@@ -1287,6 +1287,7 @@ TreeRewriterResultPtr TreeRewriter::analyzeSelect(
 
         bool is_changed = replaceAliasColumnsInQuery(query, result.storage_snapshot->metadata->getColumns(),
                                                      result.array_join_result_to_source, getContext(), excluded_nodes);
+
         /// If query is changed, we need to redo some work to correct name resolution.
         if (is_changed)
         {

src/Interpreters/TreeRewriter.h

@@ -40,11 +40,10 @@ struct TreeRewriterResult
     NameSet expanded_aliases;
 
     Aliases aliases;
-    std::vector<const ASTFunction *> aggregates;
 
+    std::vector<const ASTFunction *> aggregates;
     std::vector<const ASTFunction *> window_function_asts;
-
-    std::vector<const ASTFunction *> expressions_with_window_function;
+    ASTs expressions_with_window_function;
 
     /// Which column is needed to be ARRAY-JOIN'ed to get the specified.
     /// For example, for `SELECT s.v ... ARRAY JOIN a AS s` will get "s.v" -> "a.v".

tests/queries/0_stateless/02814_order_by_tuple_window_function.reference

@@ -0,0 +1 @@
+1

tests/queries/0_stateless/02814_order_by_tuple_window_function.sql

@@ -0,0 +1 @@
+SELECT 1 ORDER BY tuple(count() OVER ());