From b6320d4b87d2fe7b1aded6d6f89e37dbf09415cb Mon Sep 17 00:00:00 2001 From: Rebecca Taft Date: Tue, 17 Mar 2020 17:41:20 -0500 Subject: [PATCH] opt: fix some aggregate scoping issues Prior to this commit, some aggregate functions were either incorrectly rejected or incorrectly accepted when they were scoped at a higher level than their position in the query. For example, aggregate functions are not normally allowed in WHERE, but if the aggregate is actually scoped at a higher level, then the aggregate should be allowed. Prior to this commit, these aggregate functions were rejected and caused an error. This commit fixes the issue by validating the context of the aggregate's scope rather than the aggregate's position in the query. In order to avoid adding another field to the scope struct, this commit re-uses the existing `context` field which was previously only used for error messages. To make comparisons more efficient, the field is now an enum rather than a string. Fixes #44724 Fixes #45838 Fixes #30652 Release justification: This bug fix is a low risk, high benefit change to existing functionality, since it fixes internal errors and increases compatibility with Postgres. Release note (bug fix): Fixed an internal error that could occur when an aggregate inside the right-hand side of a LATERAL join was scoped at the level of the left-hand side. Release note (bug fix): Fixed an error that incorrectly occurred when an aggregate was used inside the WHERE or ON clause of a subquery but was scoped at an outer level of the query. --- pkg/sql/logictest/testdata/logic_test/join | 2 +- pkg/sql/opt/optbuilder/alter_table.go | 4 +- pkg/sql/opt/optbuilder/distinct.go | 4 +- pkg/sql/opt/optbuilder/groupby.go | 10 +- pkg/sql/opt/optbuilder/join.go | 6 +- pkg/sql/opt/optbuilder/limit.go | 4 +- pkg/sql/opt/optbuilder/orderby.go | 8 +- pkg/sql/opt/optbuilder/project.go | 8 +- pkg/sql/opt/optbuilder/scalar.go | 2 +- pkg/sql/opt/optbuilder/scope.go | 85 ++++++++++- pkg/sql/opt/optbuilder/select.go | 9 +- pkg/sql/opt/optbuilder/srfs.go | 4 +- pkg/sql/opt/optbuilder/testdata/aggregate | 156 ++++++++++++++++++++- pkg/sql/opt/optbuilder/testdata/join | 2 +- pkg/sql/opt/optbuilder/util.go | 8 +- pkg/sql/opt/optbuilder/values.go | 4 +- 16 files changed, 279 insertions(+), 37 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/join b/pkg/sql/logictest/testdata/logic_test/join index 4e9abcb2646b..0385be740f71 100644 --- a/pkg/sql/logictest/testdata/logic_test/join +++ b/pkg/sql/logictest/testdata/logic_test/join @@ -1072,7 +1072,7 @@ NULL NULL query error generator functions are not allowed in ON SELECT * FROM foo JOIN bar ON generate_series(0, 1) < 2 -query error aggregate functions are not allowed in ON +query error aggregate functions are not allowed in JOIN conditions SELECT * FROM foo JOIN bar ON max(foo.c) < 2 # Regression test for #44029 (outer join on two single-row clauses, with two diff --git a/pkg/sql/opt/optbuilder/alter_table.go b/pkg/sql/opt/optbuilder/alter_table.go index 767e695db93a..9dae428ddae7 100644 --- a/pkg/sql/opt/optbuilder/alter_table.go +++ b/pkg/sql/opt/optbuilder/alter_table.go @@ -53,12 +53,12 @@ func (b *Builder) buildAlterTableSplit(split *tree.Split, inScope *scope) (outSc // Build the expiration scalar. var expiration opt.ScalarExpr if split.ExpireExpr != nil { - emptyScope.context = "ALTER TABLE SPLIT AT" + emptyScope.context = exprKindAlterTableSplitAt // We need to save and restore the previous value of the field in // semaCtx in case we are recursively called within a subquery // context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) - b.semaCtx.Properties.Require(emptyScope.context, tree.RejectSpecial) + b.semaCtx.Properties.Require(emptyScope.context.String(), tree.RejectSpecial) texpr := emptyScope.resolveType(split.ExpireExpr, types.String) expiration = b.buildScalar(texpr, emptyScope, nil /* outScope */, nil /* outCol */, nil /* colRefs */) diff --git a/pkg/sql/opt/optbuilder/distinct.go b/pkg/sql/opt/optbuilder/distinct.go index 9dac69cd2e8a..0b384128544b 100644 --- a/pkg/sql/opt/optbuilder/distinct.go +++ b/pkg/sql/opt/optbuilder/distinct.go @@ -168,8 +168,8 @@ func (b *Builder) analyzeDistinctOnArgs( // semaCtx in case we are recursively called within a subquery // context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) - b.semaCtx.Properties.Require("DISTINCT ON", tree.RejectGenerators) - inScope.context = "DISTINCT ON" + b.semaCtx.Properties.Require(exprKindDistinctOn.String(), tree.RejectGenerators) + inScope.context = exprKindDistinctOn for i := range distinctOn { b.analyzeExtraArgument(distinctOn[i], inScope, projectionsScope, distinctOnScope) diff --git a/pkg/sql/opt/optbuilder/groupby.go b/pkg/sql/opt/optbuilder/groupby.go index 768cfa8bdcfb..9dd66d65148a 100644 --- a/pkg/sql/opt/optbuilder/groupby.go +++ b/pkg/sql/opt/optbuilder/groupby.go @@ -428,8 +428,10 @@ func (b *Builder) analyzeHaving(having *tree.Where, fromScope *scope) tree.Typed // We need to save and restore the previous value of the field in semaCtx // in case we are recursively called within a subquery context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) - b.semaCtx.Properties.Require("HAVING", tree.RejectWindowApplications|tree.RejectGenerators) - fromScope.context = "HAVING" + b.semaCtx.Properties.Require( + exprKindHaving.String(), tree.RejectWindowApplications|tree.RejectGenerators, + ) + fromScope.context = exprKindHaving return fromScope.resolveAndRequireType(having.Expr, types.Bool) } @@ -583,8 +585,8 @@ func (b *Builder) buildGrouping( defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) // Make sure the GROUP BY columns have no special functions. - b.semaCtx.Properties.Require("GROUP BY", tree.RejectSpecial) - fromScope.context = "GROUP BY" + b.semaCtx.Properties.Require(exprKindGroupBy.String(), tree.RejectSpecial) + fromScope.context = exprKindGroupBy // Resolve types, expand stars, and flatten tuples. exprs := b.expandStarAndResolveType(groupBy, fromScope) diff --git a/pkg/sql/opt/optbuilder/join.go b/pkg/sql/opt/optbuilder/join.go index c25312d9f8fa..b4c81a0e9d0f 100644 --- a/pkg/sql/opt/optbuilder/join.go +++ b/pkg/sql/opt/optbuilder/join.go @@ -90,8 +90,10 @@ func (b *Builder) buildJoin(join *tree.JoinTableExpr, inScope *scope) (outScope var filters memo.FiltersExpr if on, ok := cond.(*tree.OnJoinCond); ok { // Do not allow special functions in the ON clause. - b.semaCtx.Properties.Require("ON", tree.RejectSpecial) - outScope.context = "ON" + b.semaCtx.Properties.Require( + exprKindOn.String(), tree.RejectGenerators|tree.RejectWindowApplications, + ) + outScope.context = exprKindOn filter := b.buildScalar( outScope.resolveAndRequireType(on.Expr, types.Bool), outScope, nil, nil, nil, ) diff --git a/pkg/sql/opt/optbuilder/limit.go b/pkg/sql/opt/optbuilder/limit.go index 1d5af6316618..f6782d7221e8 100644 --- a/pkg/sql/opt/optbuilder/limit.go +++ b/pkg/sql/opt/optbuilder/limit.go @@ -26,14 +26,14 @@ func (b *Builder) buildLimit(limit *tree.Limit, parentScope, inScope *scope) { if limit.Offset != nil { input := inScope.expr.(memo.RelExpr) offset := b.resolveAndBuildScalar( - limit.Offset, types.Int, "OFFSET", tree.RejectSpecial, parentScope, + limit.Offset, types.Int, exprKindOffset, tree.RejectSpecial, parentScope, ) inScope.expr = b.factory.ConstructOffset(input, offset, inScope.makeOrderingChoice()) } if limit.Count != nil { input := inScope.expr.(memo.RelExpr) limit := b.resolveAndBuildScalar( - limit.Count, types.Int, "LIMIT", tree.RejectSpecial, parentScope, + limit.Count, types.Int, exprKindLimit, tree.RejectSpecial, parentScope, ) inScope.expr = b.factory.ConstructLimit(input, limit, inScope.makeOrderingChoice()) } diff --git a/pkg/sql/opt/optbuilder/orderby.go b/pkg/sql/opt/optbuilder/orderby.go index 9dbede3a3f99..2058110d0574 100644 --- a/pkg/sql/opt/optbuilder/orderby.go +++ b/pkg/sql/opt/optbuilder/orderby.go @@ -36,8 +36,8 @@ func (b *Builder) analyzeOrderBy( // semaCtx in case we are recursively called within a subquery // context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) - b.semaCtx.Properties.Require("ORDER BY", tree.RejectGenerators) - inScope.context = "ORDER BY" + b.semaCtx.Properties.Require(exprKindOrderBy.String(), tree.RejectGenerators) + inScope.context = exprKindOrderBy for i := range orderBy { b.analyzeOrderByArg(orderBy[i], inScope, projectionsScope, orderByScope) @@ -231,12 +231,12 @@ func (b *Builder) analyzeExtraArgument( // e.g. SELECT a, b FROM t ORDER by a+b // First, deal with projection aliases. - idx := colIdxByProjectionAlias(expr, inScope.context, projectionsScope) + idx := colIdxByProjectionAlias(expr, inScope.context.String(), projectionsScope) // If the expression does not refer to an alias, deal with // column ordinals. if idx == -1 { - idx = colIndex(len(projectionsScope.cols), expr, inScope.context) + idx = colIndex(len(projectionsScope.cols), expr, inScope.context.String()) } var exprs tree.TypedExprs diff --git a/pkg/sql/opt/optbuilder/project.go b/pkg/sql/opt/optbuilder/project.go index 923f4842b501..cb3d4e60ce3f 100644 --- a/pkg/sql/opt/optbuilder/project.go +++ b/pkg/sql/opt/optbuilder/project.go @@ -70,8 +70,8 @@ func (b *Builder) analyzeProjectionList( defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) defer func(replaceSRFs bool) { inScope.replaceSRFs = replaceSRFs }(inScope.replaceSRFs) - b.semaCtx.Properties.Require("SELECT", tree.RejectNestedGenerators) - inScope.context = "SELECT" + b.semaCtx.Properties.Require(exprKindSelect.String(), tree.RejectNestedGenerators) + inScope.context = exprKindSelect inScope.replaceSRFs = true b.analyzeSelectList(selects, desiredTypes, inScope, outScope) @@ -89,8 +89,8 @@ func (b *Builder) analyzeReturningList( defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) // Ensure there are no special functions in the RETURNING clause. - b.semaCtx.Properties.Require("RETURNING", tree.RejectSpecial) - inScope.context = "RETURNING" + b.semaCtx.Properties.Require(exprKindReturning.String(), tree.RejectSpecial) + inScope.context = exprKindReturning b.analyzeSelectList(tree.SelectExprs(returning), desiredTypes, inScope, outScope) } diff --git a/pkg/sql/opt/optbuilder/scalar.go b/pkg/sql/opt/optbuilder/scalar.go index 7e2f4745222d..b266955c96bf 100644 --- a/pkg/sql/opt/optbuilder/scalar.go +++ b/pkg/sql/opt/optbuilder/scalar.go @@ -573,7 +573,7 @@ func (b *Builder) checkSubqueryOuterCols( aggCols := inScope.groupby.aggregateResultCols() for i := range aggCols { if subqueryOuterCols.Contains(aggCols[i].id) { - panic(tree.NewInvalidFunctionUsageError(tree.AggregateClass, inScope.context)) + panic(tree.NewInvalidFunctionUsageError(tree.AggregateClass, inScope.context.String())) } } } diff --git a/pkg/sql/opt/optbuilder/scope.go b/pkg/sql/opt/optbuilder/scope.go index 1bc82582e489..61fdae90a5be 100644 --- a/pkg/sql/opt/optbuilder/scope.go +++ b/pkg/sql/opt/optbuilder/scope.go @@ -99,8 +99,10 @@ type scope struct { ctes map[string]*cteSource // context is the current context in the SQL query (e.g., "SELECT" or - // "HAVING"). It is used for error messages. - context string + // "HAVING"). It is used for error messages and to identify scoping errors + // (e.g., aggregates are not allowed in the FROM clause of their own query + // level). + context exprKind } // cteSource represents a CTE in the given query. @@ -112,6 +114,57 @@ type cteSource struct { id opt.WithID } +// exprKind is used to represent the kind of the current expression in the +// SQL query. +type exprKind int8 + +const ( + exprKindNone exprKind = iota + exprKindAlterTableSplitAt + exprKindDistinctOn + exprKindFrom + exprKindGroupBy + exprKindHaving + exprKindLateralJoin + exprKindLimit + exprKindOffset + exprKindOn + exprKindOrderBy + exprKindReturning + exprKindSelect + exprKindValues + exprKindWhere + exprKindWindowFrameStart + exprKindWindowFrameEnd +) + +var exprKindName = [...]string{ + exprKindNone: "", + exprKindAlterTableSplitAt: "ALTER TABLE SPLIT AT", + exprKindDistinctOn: "DISTINCT ON", + exprKindFrom: "FROM", + exprKindGroupBy: "GROUP BY", + exprKindHaving: "HAVING", + exprKindLateralJoin: "LATERAL JOIN", + exprKindLimit: "LIMIT", + exprKindOffset: "OFFSET", + exprKindOn: "ON", + exprKindOrderBy: "ORDER BY", + exprKindReturning: "RETURNING", + exprKindSelect: "SELECT", + exprKindValues: "VALUES", + exprKindWhere: "WHERE", + exprKindWindowFrameStart: "WINDOW FRAME START", + exprKindWindowFrameEnd: "WINDOW FRAME END", +} + +func (k exprKind) String() string { + if k < 0 || k > exprKind(len(exprKindName)-1) { + return fmt.Sprintf("exprKind(%d)", k) + } + return exprKindName[k] +} + // initGrouping initializes the groupby information for this scope. func (s *scope) initGrouping() { if s.groupby != nil { @@ -329,7 +382,7 @@ func (s *scope) resolveType(expr tree.Expr, desired *types.T) tree.TypedExpr { // desired type. func (s *scope) resolveAndRequireType(expr tree.Expr, desired *types.T) tree.TypedExpr { expr = s.walkExprTree(expr) - texpr, err := tree.TypeCheckAndRequire(expr, s.builder.semaCtx, desired, s.context) + texpr, err := tree.TypeCheckAndRequire(expr, s.builder.semaCtx, desired, s.context.String()) if err != nil { panic(err) } @@ -510,6 +563,7 @@ func (s *scope) endAggFunc(cols opt.ColSet) (g *groupby) { for curr := s; curr != nil; curr = curr.parent { if cols.Len() == 0 || cols.Intersects(curr.colSet()) { + curr.verifyAggregateContext() if curr.groupby == nil { curr.initGrouping() } @@ -520,6 +574,25 @@ func (s *scope) endAggFunc(cols opt.ColSet) (g *groupby) { panic(errors.AssertionFailedf("aggregate function is not allowed in this context")) } +// verifyAggregateContext checks that the current scope is allowed to contain +// aggregate functions. +func (s *scope) verifyAggregateContext() { + switch s.context { + case exprKindLateralJoin: + panic(pgerror.Newf(pgcode.Grouping, + "aggregate functions are not allowed in FROM clause of their own query level", + )) + + case exprKindOn: + panic(pgerror.Newf(pgcode.Grouping, + "aggregate functions are not allowed in JOIN conditions", + )) + + case exprKindWhere: + panic(tree.NewInvalidFunctionUsageError(tree.AggregateClass, s.context.String())) + } +} + // scope implements the tree.Visitor interface so that it can walk through // a tree.Expr tree, perform name resolution, and replace unresolved column // names with a scopeColumn. The info stored in scopeColumn is necessary for @@ -858,7 +931,7 @@ func (s *scope) replaceSRF(f *tree.FuncExpr, def *tree.FunctionDefinition) *srf // context. defer s.builder.semaCtx.Properties.Restore(s.builder.semaCtx.Properties) - s.builder.semaCtx.Properties.Require(s.context, + s.builder.semaCtx.Properties.Require(s.context.String(), tree.RejectAggregates|tree.RejectWindowApplications|tree.RejectNestedGenerators) expr := f.Walk(s) @@ -1104,13 +1177,13 @@ func analyzeWindowFrame(s *scope, windowDef *tree.WindowDef) error { } if startBound != nil && startBound.OffsetExpr != nil { oldContext := s.context - s.context = "WINDOW FRAME START" + s.context = exprKindWindowFrameStart startBound.OffsetExpr = s.resolveAndRequireType(startBound.OffsetExpr, requiredType) s.context = oldContext } if endBound != nil && endBound.OffsetExpr != nil { oldContext := s.context - s.context = "WINDOW FRAME END" + s.context = exprKindWindowFrameEnd endBound.OffsetExpr = s.resolveAndRequireType(endBound.OffsetExpr, requiredType) s.context = oldContext } diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go index d38ec5c2283a..97367a313ad7 100644 --- a/pkg/sql/opt/optbuilder/select.go +++ b/pkg/sql/opt/optbuilder/select.go @@ -918,7 +918,13 @@ func (b *Builder) buildWhere(where *tree.Where, inScope *scope) { return } - filter := b.resolveAndBuildScalar(where.Expr, types.Bool, "WHERE", tree.RejectSpecial, inScope) + filter := b.resolveAndBuildScalar( + where.Expr, + types.Bool, + exprKindWhere, + tree.RejectGenerators|tree.RejectWindowApplications, + inScope, + ) // Wrap the filter in a FiltersOp. inScope.expr = b.factory.ConstructSelect( @@ -1017,6 +1023,7 @@ func (b *Builder) buildFromWithLateral(tables tree.TableExprs, inScope *scope) ( // have been built already. if b.exprIsLateral(tables[i]) { scope = outScope + scope.context = exprKindLateralJoin } tableScope := b.buildDataSource(tables[i], nil /* indexFlags */, scope) diff --git a/pkg/sql/opt/optbuilder/srfs.go b/pkg/sql/opt/optbuilder/srfs.go index 318579e0709a..ebad0a9bf5cb 100644 --- a/pkg/sql/opt/optbuilder/srfs.go +++ b/pkg/sql/opt/optbuilder/srfs.go @@ -75,9 +75,9 @@ func (b *Builder) buildZip(exprs tree.Exprs, inScope *scope) (outScope *scope) { // semaCtx in case we are recursively called within a subquery // context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) - b.semaCtx.Properties.Require("FROM", + b.semaCtx.Properties.Require(exprKindFrom.String(), tree.RejectAggregates|tree.RejectWindowApplications|tree.RejectNestedGenerators) - inScope.context = "FROM" + inScope.context = exprKindFrom // Build each of the provided expressions. zip := make(memo.ZipExpr, len(exprs)) diff --git a/pkg/sql/opt/optbuilder/testdata/aggregate b/pkg/sql/opt/optbuilder/testdata/aggregate index c71bfdbc89ad..a40254898c1c 100644 --- a/pkg/sql/opt/optbuilder/testdata/aggregate +++ b/pkg/sql/opt/optbuilder/testdata/aggregate @@ -564,7 +564,7 @@ error (42803): column "v" must appear in the GROUP BY clause or be used in an ag build SELECT k FROM kv WHERE avg(k) > 1 ---- -error (42803): avg(): aggregate functions are not allowed in WHERE +error (42803): aggregate functions are not allowed in WHERE build SELECT max(avg(k)) FROM kv @@ -3881,3 +3881,157 @@ project │ └── variable: column5 [type=int] └── sum [type=decimal] └── variable: column7 [type=int] + +# Regression test for #44724. +build +SELECT * + FROM (SELECT 1 AS one, v FROM kv) AS kv, + LATERAL ( + SELECT b, sum(one) FROM abxy + ) AS abxy WHERE kv.v = abxy.b +---- +error (42803): aggregate functions are not allowed in FROM clause of their own query level + +# Regression test for #45838. The aggregate should be allowed in the WHERE +# clause since it's scoped at the outer level. +build + SELECT sum(x) + FROM abxy AS t +GROUP BY y + HAVING EXISTS(SELECT 1 FROM abxy AS t2 WHERE sum(t.x) = 1) +---- +project + ├── columns: sum:5(decimal) + └── select + ├── columns: t.y:4(int) sum:5(decimal) + ├── group-by + │ ├── columns: t.y:4(int) sum:5(decimal) + │ ├── grouping columns: t.y:4(int) + │ ├── project + │ │ ├── columns: t.x:3(int) t.y:4(int) + │ │ └── scan t + │ │ └── columns: t.a:1(int!null) t.b:2(int!null) t.x:3(int) t.y:4(int) + │ └── aggregations + │ └── sum [type=decimal] + │ └── variable: t.x [type=int] + └── filters + └── exists [type=bool] + └── project + ├── columns: "?column?":11(int!null) + ├── select + │ ├── columns: t2.a:6(int!null) t2.b:7(int!null) t2.x:8(int) t2.y:9(int) + │ ├── scan t2 + │ │ └── columns: t2.a:6(int!null) t2.b:7(int!null) t2.x:8(int) t2.y:9(int) + │ └── filters + │ └── eq [type=bool] + │ ├── variable: sum [type=decimal] + │ └── const: 1 [type=decimal] + └── projections + └── const: 1 [type=int] + +exec-ddl +CREATE TABLE onek ( + unique1 int, + unique2 int, + two int, + four int, + ten int, + twenty int, + hundred int, + thousand int, + twothousand int, + fivethous int, + tenthous int, + odd int, + even int, + stringu1 string, + stringu2 string, + string4 string +) +---- + +# Regression tests for #30652. +build + SELECT ten, sum(DISTINCT four) + FROM onek AS a +GROUP BY ten + HAVING EXISTS( + SELECT 1 FROM onek AS b WHERE sum(DISTINCT a.four) = b.four + ) +---- +select + ├── columns: ten:5(int) sum:18(decimal) + ├── group-by + │ ├── columns: a.ten:5(int) sum:18(decimal) + │ ├── grouping columns: a.ten:5(int) + │ ├── project + │ │ ├── columns: a.four:4(int) a.ten:5(int) + │ │ └── scan a + │ │ └── columns: a.unique1:1(int) a.unique2:2(int) a.two:3(int) a.four:4(int) a.ten:5(int) a.twenty:6(int) a.hundred:7(int) a.thousand:8(int) a.twothousand:9(int) a.fivethous:10(int) a.tenthous:11(int) a.odd:12(int) a.even:13(int) a.stringu1:14(string) a.stringu2:15(string) a.string4:16(string) a.rowid:17(int!null) + │ └── aggregations + │ └── sum [type=decimal] + │ └── agg-distinct [type=int] + │ └── variable: a.four [type=int] + └── filters + └── exists [type=bool] + └── project + ├── columns: "?column?":37(int!null) + ├── select + │ ├── columns: b.unique1:19(int) b.unique2:20(int) b.two:21(int) b.four:22(int!null) b.ten:23(int) b.twenty:24(int) b.hundred:25(int) b.thousand:26(int) b.twothousand:27(int) b.fivethous:28(int) b.tenthous:29(int) b.odd:30(int) b.even:31(int) b.stringu1:32(string) b.stringu2:33(string) b.string4:34(string) b.rowid:35(int!null) + │ ├── scan b + │ │ └── columns: b.unique1:19(int) b.unique2:20(int) b.two:21(int) b.four:22(int) b.ten:23(int) b.twenty:24(int) b.hundred:25(int) b.thousand:26(int) b.twothousand:27(int) b.fivethous:28(int) b.tenthous:29(int) b.odd:30(int) b.even:31(int) b.stringu1:32(string) b.stringu2:33(string) b.string4:34(string) b.rowid:35(int!null) + │ └── filters + │ └── eq [type=bool] + │ ├── variable: sum [type=decimal] + │ └── variable: b.four [type=int] + └── projections + └── const: 1 [type=int] + +build + SELECT ten, sum(DISTINCT four) + FROM onek AS a +GROUP BY ten + HAVING EXISTS( + SELECT 1 + FROM onek AS b + WHERE sum(DISTINCT a.four + b.four) = b.four + ) +---- +error (42803): aggregate functions are not allowed in WHERE + +build +SELECT ( + SELECT t2.a + FROM abxy AS t2 JOIN abxy AS t3 ON sum(t1.x) = t3.x + ) + FROM abxy AS t1 +---- +project + ├── columns: a:15(int) + ├── scalar-group-by + │ ├── columns: sum:14(decimal) + │ ├── project + │ │ ├── columns: x:13(int) + │ │ ├── scan t1 + │ │ │ └── columns: t1.a:1(int!null) t1.b:2(int!null) t1.x:3(int) t1.y:4(int) + │ │ └── projections + │ │ └── variable: t1.x [type=int] + │ └── aggregations + │ └── sum [type=decimal] + │ └── variable: x [type=int] + └── projections + └── subquery [type=int] + └── max1-row + ├── columns: t2.a:5(int!null) + └── project + ├── columns: t2.a:5(int!null) + └── inner-join (cross) + ├── columns: t2.a:5(int!null) t2.b:6(int!null) t2.x:7(int) t2.y:8(int) t3.a:9(int!null) t3.b:10(int!null) t3.x:11(int!null) t3.y:12(int) + ├── scan t2 + │ └── columns: t2.a:5(int!null) t2.b:6(int!null) t2.x:7(int) t2.y:8(int) + ├── scan t3 + │ └── columns: t3.a:9(int!null) t3.b:10(int!null) t3.x:11(int) t3.y:12(int) + └── filters + └── eq [type=bool] + ├── variable: sum [type=decimal] + └── variable: t3.x [type=int] diff --git a/pkg/sql/opt/optbuilder/testdata/join b/pkg/sql/opt/optbuilder/testdata/join index eeeaa6f6d4cd..abebdf810060 100644 --- a/pkg/sql/opt/optbuilder/testdata/join +++ b/pkg/sql/opt/optbuilder/testdata/join @@ -3020,7 +3020,7 @@ error (0A000): generate_series(): generator functions are not allowed in ON build SELECT * FROM foo JOIN bar ON max(foo.c) < 2 ---- -error (42803): max(): aggregate functions are not allowed in ON +error (42803): aggregate functions are not allowed in JOIN conditions # Verify join hints get populated. build diff --git a/pkg/sql/opt/optbuilder/util.go b/pkg/sql/opt/optbuilder/util.go index b9a0926cbf12..5039bb8b249b 100644 --- a/pkg/sql/opt/optbuilder/util.go +++ b/pkg/sql/opt/optbuilder/util.go @@ -398,13 +398,17 @@ func colsToColList(cols []scopeColumn) opt.ColList { // resolveAndBuildScalar is used to build a scalar with a required type. func (b *Builder) resolveAndBuildScalar( - expr tree.Expr, requiredType *types.T, context string, flags tree.SemaRejectFlags, inScope *scope, + expr tree.Expr, + requiredType *types.T, + context exprKind, + flags tree.SemaRejectFlags, + inScope *scope, ) opt.ScalarExpr { // We need to save and restore the previous value of the field in // semaCtx in case we are recursively called within a subquery // context. defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) - b.semaCtx.Properties.Require(context, flags) + b.semaCtx.Properties.Require(context.String(), flags) inScope.context = context texpr := inScope.resolveAndRequireType(expr, requiredType) diff --git a/pkg/sql/opt/optbuilder/values.go b/pkg/sql/opt/optbuilder/values.go index 358882036463..3e11a0fe1333 100644 --- a/pkg/sql/opt/optbuilder/values.go +++ b/pkg/sql/opt/optbuilder/values.go @@ -40,8 +40,8 @@ func (b *Builder) buildValuesClause( defer b.semaCtx.Properties.Restore(b.semaCtx.Properties) // Ensure there are no special functions in the clause. - b.semaCtx.Properties.Require("VALUES", tree.RejectSpecial) - inScope.context = "VALUES" + b.semaCtx.Properties.Require(exprKindValues.String(), tree.RejectSpecial) + inScope.context = exprKindValues // Typing a VALUES clause is not trivial; consider: // VALUES (NULL), (1)