Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #52145 to 23.3: Fix ORDER BY tuple of WINDOW functions #52165

Merged
merged 1 commit into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 5 additions & 6 deletions src/Interpreters/ExpressionAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1457,10 +1457,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 */)
Expand Down Expand Up @@ -1839,9 +1838,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();
Expand Down Expand Up @@ -1884,7 +1883,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();
Expand Down
20 changes: 14 additions & 6 deletions src/Interpreters/GetAggregatesVisitor.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#include <Interpreters/GetAggregatesVisitor.h>
#include <Common/checkStackSize.h>


namespace DB
{
Expand All @@ -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.
//
Expand All @@ -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;
}

Expand All @@ -50,6 +55,8 @@ struct WindowExpressionsCollectorMatcher
ASTPtr & ast,
const ASTPtr & parent)
{
checkStackSize();

if (auto * func = ast->as<ASTFunction>())
{
if (func->is_window_function)
Expand All @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Interpreters/GetAggregatesVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
1 change: 1 addition & 0 deletions src/Interpreters/TreeRewriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1283,6 +1283,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)
{
Expand Down
5 changes: 2 additions & 3 deletions src/Interpreters/TreeRewriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SELECT 1 ORDER BY tuple(count() OVER ());