Merge #38623

38623: sqlfmt: retain ORDER BY in aggregates r=justinj a=justinj Release note (bug fix): sqlfmt no longer strips ORDER BY from aggregates. Co-authored-by: Justin Jaffray <justin@cockroachlabs.com>
cockroachdb · Jul 3, 2019 · 393a6f1 · 393a6f1
2 parents 69fc2b9 + a5f6f04
commit 393a6f1
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 0 deletions.
diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go
@@ -839,6 +839,8 @@ func TestParse(t *testing.T) {
 
 		{`SELECT 1 FROM t GROUP BY a`},
 		{`SELECT 1 FROM t GROUP BY a, b`},
+		{`SELECT sum(x ORDER BY y) FROM t`},
+		{`SELECT sum(x ORDER BY y, z) FROM t`},
 
 		{`SELECT a FROM t HAVING a = b`},
 

diff --git a/pkg/sql/sem/tree/expr.go b/pkg/sql/sem/tree/expr.go
@@ -1354,6 +1354,10 @@ func (node *FuncExpr) Format(ctx *FmtCtx) {
 	ctx.WriteByte('(')
 	ctx.WriteString(typ)
 	ctx.FormatNode(&node.Exprs)
+	if len(node.OrderBy) > 0 {
+		ctx.WriteByte(' ')
+		ctx.FormatNode(&node.OrderBy)
+	}
 	ctx.WriteByte(')')
 	if ctx.HasFlags(FmtParsable) && node.typ != nil {
 		if node.fnProps.AmbiguousReturnType {

diff --git a/pkg/sql/sem/tree/pretty.go b/pkg/sql/sem/tree/pretty.go
@@ -679,6 +679,10 @@ func (node *FuncExpr) doc(p *PrettyCfg) pretty.Doc {
 				args,
 			)
 		}
+
+		if len(node.OrderBy) > 0 {
+			args = pretty.ConcatSpace(args, node.OrderBy.doc(p))
+		}
 		d = pretty.Concat(d, p.bracket("(", args, ")"))
 	} else {
 		d = pretty.Concat(d, pretty.Text("()"))

diff --git a/pkg/sql/sem/tree/testdata/pretty/functions.align-deindent.golden.short b/pkg/sql/sem/tree/testdata/pretty/functions.align-deindent.golden.short
@@ -3,6 +3,9 @@
 1:
 -
 SELECT min(a, b),
+       min(a ORDER BY b),
+       min(a ORDER BY b, c),
+       min(a ORDER BY b, c, d),
        min(DISTINCT a, b),
        min(),
        min() OVER (),

diff --git a/pkg/sql/sem/tree/testdata/pretty/functions.align-only.golden.short b/pkg/sql/sem/tree/testdata/pretty/functions.align-only.golden.short
@@ -3,6 +3,9 @@
 1:
 -
 SELECT min(a, b),
+       min(a ORDER BY b),
+       min(a ORDER BY b, c),
+       min(a ORDER BY b, c, d),
        min(DISTINCT a, b),
        min(),
        min() OVER (),

diff --git a/pkg/sql/sem/tree/testdata/pretty/functions.ref.golden.short b/pkg/sql/sem/tree/testdata/pretty/functions.ref.golden.short
@@ -4,6 +4,9 @@
 -
 SELECT
 	min(a, b),
+	min(a ORDER BY b),
+	min(a ORDER BY b, c),
+	min(a ORDER BY b, c, d),
 	min(DISTINCT a, b),
 	min(),
 	min() OVER (),

diff --git a/pkg/sql/sem/tree/testdata/pretty/functions.sql b/pkg/sql/sem/tree/testdata/pretty/functions.sql
@@ -1,5 +1,8 @@
 select
 	min(a,b),
+	min(a order by b),
+	min(a order by b, c),
+	min(a order by b, c, d),
 	min(distinct a,b),
 	min(),
 	min() over (),