From 4cc02b7aecbceb1829960e6b014646ce01b00e87 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Thu, 25 Apr 2019 10:40:40 -0700 Subject: [PATCH] sql: fix checking for presence of window functions in some edge cases Previously, when a window function was a part of another expression (say BinaryExpr) instead of being used directly within PARTITION BY and ORDER BY clauses of another window function, we would incorrectly not reject such a query and would actually crash while trying to execute it. Release note: None --- pkg/sql/logictest/testdata/logic_test/window | 6 ++++++ pkg/sql/window.go | 14 ++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/window b/pkg/sql/logictest/testdata/logic_test/window index 0ab8f2b7df5c..85a8d25bbd97 100644 --- a/pkg/sql/logictest/testdata/logic_test/window +++ b/pkg/sql/logictest/testdata/logic_test/window @@ -3008,6 +3008,12 @@ SELECT sum(a) OVER (PARTITION BY count(a) OVER ()) FROM x statement error window functions are not allowed in window definitions SELECT sum(a) OVER (ORDER BY count(a) OVER ()) FROM x +statement error window functions are not allowed in window definitions +SELECT sum(a) OVER (PARTITION BY count(a) OVER () + 1) FROM x + +statement error window functions are not allowed in window definitions +SELECT sum(a) OVER (ORDER BY count(a) OVER () + 1) FROM x + statement error more than one row returned by a subquery used as an expression SELECT sum(a) OVER (PARTITION BY (SELECT count(*) FROM x GROUP BY a)) FROM x diff --git a/pkg/sql/window.go b/pkg/sql/window.go index 1c50027d582e..cdfca85e83a5 100644 --- a/pkg/sql/window.go +++ b/pkg/sql/window.go @@ -179,6 +179,8 @@ func (n *windowNode) extractWindowFunctions(s *renderNode) error { func (p *planner) constructWindowDefinitions( ctx context.Context, n *windowNode, sc *tree.SelectClause, s *renderNode, ) error { + var containsWindowVisitor transform.ContainsWindowVisitor + // Process each named window specification on the select clause. namedWindowSpecs := make(map[string]*tree.WindowDef, len(sc.Window)) for _, windowDef := range sc.Window { @@ -224,10 +226,8 @@ func (p *planner) constructWindowDefinitions( // Validate PARTITION BY clause. for _, partition := range windowDef.Partitions { - if funcExpr, ok := partition.(*tree.FuncExpr); ok { - if funcExpr.IsWindowFunctionApplication() { - return pgerror.Newf(pgerror.CodeWindowingError, "window functions are not allowed in window definitions") - } + if containsWindowVisitor.ContainsWindowFunc(partition) { + return pgerror.Newf(pgerror.CodeWindowingError, "window functions are not allowed in window definitions") } cols, exprs, _, err := p.computeRenderAllowingStars(ctx, tree.SelectExpr{Expr: partition}, types.Any, s.sourceInfo, s.ivarHelper, @@ -242,10 +242,8 @@ func (p *planner) constructWindowDefinitions( // Validate ORDER BY clause. for _, orderBy := range windowDef.OrderBy { - if funcExpr, ok := orderBy.Expr.(*tree.FuncExpr); ok { - if funcExpr.IsWindowFunctionApplication() { - return pgerror.Newf(pgerror.CodeWindowingError, "window functions are not allowed in window definitions") - } + if containsWindowVisitor.ContainsWindowFunc(orderBy.Expr) { + return pgerror.Newf(pgerror.CodeWindowingError, "window functions are not allowed in window definitions") } cols, exprs, _, err := p.computeRenderAllowingStars(ctx, tree.SelectExpr{Expr: orderBy.Expr}, types.Any, s.sourceInfo, s.ivarHelper,