sql: disallow aggregate functions in ORDER BY in DELETE and UPDATE

This commit disallows aggregate functions in the context of an
`ORDER BY` clause in a `DELETE` or `UPDATE` statement. An aggregate
function in an `ORDER BY` would require a `GROUP BY` clause to group
non-aggregate columns. A `GROUP BY` is not allowed in `DELETE` or
`UPDATE` statements as it's not obvious how grouping in these statements
would behave. So we simply disallow aggregates in `ORDER BY` instead.

Fixes cockroachdb#107634

Release note (bug fix): A bug has been fixed that caused internal errors
when using an aggregate function in an `ORDER BY` clause of a `DELETE`
or `UPDATE` statement. Aggregate functions are no longer allowed in
these contexts. The bug has been present since at least version 20.2.

Author: mgartner

pkg/sql/logictest/testdata/logic_test/delete

@@ -565,3 +565,14 @@ false
 query I
 SELECT a FROM t99630a@idx WHERE a > 0
 ----
+
+# Regression test for #107634. Do not allow aggregate functions in ORDER BY.
+subtest regression_107634
+
+statement ok
+CREATE TABLE t107634 (a INT)
+
+statement error pgcode 42803 sum\(\): aggregate functions are not allowed in ORDER BY in DELETE
+DELETE FROM t107634 ORDER BY sum(a) LIMIT 1;
+
+subtest end

pkg/sql/logictest/testdata/logic_test/update

@@ -661,3 +661,14 @@ SELECT * FROM generated_as_id_t ORDER BY a;
 7  4  4
 8  5  5
 9  6  6
+
+# Regression test for #107634. Do not allow aggregate functions in ORDER BY.
+subtest regression_107634
+
+statement ok
+CREATE TABLE t107634 (a INT)
+
+statement error pgcode 42803 sum\(\): aggregate functions are not allowed in ORDER BY in UPDATE
+UPDATE t107634 SET a = 1 ORDER BY sum(a) LIMIT 1;
+
+subtest end

pkg/sql/opt/optbuilder/mutation_builder.go

@@ -336,7 +336,8 @@ func (mb *mutationBuilder) buildInputForUpdate(
 	// SELECT + ORDER BY (which may add projected expressions)
 	projectionsScope := mb.outScope.replace()
 	projectionsScope.appendColumnsFromScope(mb.outScope)
-	orderByScope := mb.b.analyzeOrderBy(orderBy, mb.outScope, projectionsScope, tree.RejectGenerators)
+	orderByScope := mb.b.analyzeOrderBy(orderBy, mb.outScope, projectionsScope,
+		exprKindOrderByUpdate, tree.RejectGenerators|tree.RejectAggregates)
 	mb.b.buildOrderBy(mb.outScope, projectionsScope, orderByScope)
 	mb.b.constructProjectForScope(mb.outScope, projectionsScope)
 
@@ -448,7 +449,8 @@ func (mb *mutationBuilder) buildInputForDelete(
 	// SELECT + ORDER BY (which may add projected expressions)
 	projectionsScope := mb.outScope.replace()
 	projectionsScope.appendColumnsFromScope(mb.outScope)
-	orderByScope := mb.b.analyzeOrderBy(orderBy, mb.outScope, projectionsScope, tree.RejectGenerators)
+	orderByScope := mb.b.analyzeOrderBy(orderBy, mb.outScope, projectionsScope,
+		exprKindOrderByDelete, tree.RejectGenerators|tree.RejectAggregates)
 	mb.b.buildOrderBy(mb.outScope, projectionsScope, orderByScope)
 	mb.b.constructProjectForScope(mb.outScope, projectionsScope)
 

pkg/sql/opt/optbuilder/orderby.go

@@ -28,7 +28,10 @@ import (
 // analyzeOrderBy analyzes an Ordering physical property from the ORDER BY
 // clause and adds the resulting typed expressions to orderByScope.
 func (b *Builder) analyzeOrderBy(
-	orderBy tree.OrderBy, inScope, projectionsScope *scope, rejectFlags tree.SemaRejectFlags,
+	orderBy tree.OrderBy,
+	inScope, projectionsScope *scope,
+	kind exprKind,
+	rejectFlags tree.SemaRejectFlags,
 ) (orderByScope *scope) {
 	if orderBy == nil {
 		return nil
@@ -41,8 +44,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(exprKindOrderBy.String(), rejectFlags)
-	inScope.context = exprKindOrderBy
+	b.semaCtx.Properties.Require(kind.String(), rejectFlags)
+	inScope.context = kind
 
 	for i := range orderBy {
 		b.analyzeOrderByArg(orderBy[i], inScope, projectionsScope, orderByScope)

pkg/sql/opt/optbuilder/scope.go

@@ -132,6 +132,8 @@ const (
 	exprKindOffset
 	exprKindOn
 	exprKindOrderBy
+	exprKindOrderByDelete
+	exprKindOrderByUpdate
 	exprKindReturning
 	exprKindSelect
 	exprKindStoreID
@@ -153,6 +155,8 @@ var exprKindName = [...]string{
 	exprKindOffset:            "OFFSET",
 	exprKindOn:                "ON",
 	exprKindOrderBy:           "ORDER BY",
+	exprKindOrderByDelete:     "ORDER BY in DELETE",
+	exprKindOrderByUpdate:     "ORDER BY in UPDATE",
 	exprKindReturning:         "RETURNING",
 	exprKindSelect:            "SELECT",
 	exprKindStoreID:           "RELOCATE STORE ID",

pkg/sql/opt/optbuilder/select.go

@@ -1106,7 +1106,8 @@ func (b *Builder) buildSelectStmtWithoutParens(
 			col := projectionsScope.addColumn(scopeColName(""), expr)
 			b.buildScalar(expr, outScope, projectionsScope, col, nil)
 		}
-		orderByScope := b.analyzeOrderBy(orderBy, outScope, projectionsScope, tree.RejectGenerators|tree.RejectAggregates|tree.RejectWindowApplications)
+		orderByScope := b.analyzeOrderBy(orderBy, outScope, projectionsScope, exprKindOrderBy,
+			tree.RejectGenerators|tree.RejectAggregates|tree.RejectWindowApplications)
 		b.buildOrderBy(outScope, projectionsScope, orderByScope)
 		b.constructProjectForScope(outScope, projectionsScope)
 		outScope = projectionsScope
@@ -1151,7 +1152,8 @@ func (b *Builder) buildSelectClause(
 	// Any aggregates in the HAVING, ORDER BY and DISTINCT ON clauses (if they
 	// exist) will be added here.
 	havingExpr := b.analyzeHaving(sel.Having, fromScope)
-	orderByScope := b.analyzeOrderBy(orderBy, fromScope, projectionsScope, tree.RejectGenerators)
+	orderByScope := b.analyzeOrderBy(orderBy, fromScope, projectionsScope,
+		exprKindOrderBy, tree.RejectGenerators)
 	distinctOnScope := b.analyzeDistinctOnArgs(sel.DistinctOn, fromScope, projectionsScope)
 
 	var having opt.ScalarExpr

pkg/sql/opt/optbuilder/testdata/delete

@@ -299,6 +299,12 @@ DELETE FROM abcde WHERE b=1 ORDER BY c
 ----
 error (42601): DELETE statement requires LIMIT when ORDER BY is used
 
+# Aggregate functions are not allowed in ORDER BY.
+build
+DELETE FROM abcde WHERE b=1 ORDER BY sum(c) LIMIT 1
+----
+error (42803): sum(): aggregate functions are not allowed in ORDER BY in DELETE
+
 # ------------------------------------------------------------------------------
 # Test RETURNING.
 # ------------------------------------------------------------------------------

pkg/sql/opt/optbuilder/testdata/update

@@ -617,6 +617,12 @@ UPDATE abcde SET b=1 ORDER BY c
 ----
 error (42601): UPDATE statement requires LIMIT when ORDER BY is used
 
+# Aggregate functions are not allowed in ORDER BY.
+build
+UPDATE abcde SET b=1 ORDER BY sum(c) LIMIT 1
+----
+error (42803): sum(): aggregate functions are not allowed in ORDER BY in UPDATE
+
 # ------------------------------------------------------------------------------
 # Test RETURNING.
 # ------------------------------------------------------------------------------