Merge pull request #19569 from jordanlewis/anyallsome-fmt

sql: disambiguate ANY/ALL/SOME exprs in formatting
cockroachdb · Oct 26, 2017 · 5a6b431 · 5a6b431
2 parents c039a09 + 5a729ee
commit 5a6b431
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 14 deletions.
diff --git a/pkg/sql/index_selection_test.go b/pkg/sql/index_selection_test.go
@@ -743,7 +743,7 @@ func TestApplyConstraints(t *testing.T) {
 		// doesn't decompose further.
 		{`a < 3 AND (b < 2 OR a IN (0,1,2))`, `a`, `(b < 2) OR (a IN (0, 1, 2))`},
 		{`a < 3 AND (b < 2 OR a NOT IN (0,1,2))`, `a`, `(b < 2) OR (a NOT IN (0, 1, 2))`},
-		{`a < 3 AND (b < 2 OR a = ANY ARRAY[0,1,2])`, `a`, `(b < 2) OR (a = ANY ARRAY[0,1,2])`},
+		{`a < 3 AND (b < 2 OR a = ANY ARRAY[0,1,2])`, `a`, `(b < 2) OR (a = ANY (ARRAY[0,1,2]))`},
 		{`a IN (0, 2, 3) AND (b < 2 OR a <= 4)`, `a`, `(b < 2) OR (a <= 4)`},
 		{`a IN (0, 2, 3) AND (b < 2 OR a = 2)`, `a`, `(b < 2) OR (a = 2)`},
 	}

diff --git a/pkg/sql/parser/expr.go b/pkg/sql/parser/expr.go
@@ -158,7 +158,9 @@ func binExprFmtWithParenAndSubOp(
 	}
 	buf.WriteString(op)
 	buf.WriteByte(' ')
-	exprFmtWithParen(buf, f, e2)
+	buf.WriteByte('(')
+	FormatNode(buf, f, StripParens(e2))
+	buf.WriteByte(')')
 }
 
 // Format implements the NodeFormatter interface.

diff --git a/pkg/sql/parser/normalize_test.go b/pkg/sql/parser/normalize_test.go
@@ -98,10 +98,10 @@ func TestNormalizeExpr(t *testing.T) {
 		{`NULL > SOME ARRAY[3, 2, 1]`, `NULL`},
 		{`NULL > ALL ARRAY[3, 2, 1]`, `NULL`},
 		{`4 > ALL ARRAY[3, 2, 1]`, `true`},
-		{`a > ALL ARRAY[3, 2, 1]`, `a > ALL ARRAY[3,2,1]`},
-		{`3 > ALL ARRAY[3, 2, a]`, `3 > ALL ARRAY[3, 2, a]`},
-		{`3 > ANY (ARRAY[3, 2, a])`, `3 > ANY ARRAY[3, 2, a]`},
-		{`3 > SOME (((ARRAY[3, 2, a])))`, `3 > SOME ARRAY[3, 2, a]`},
+		{`a > ALL ARRAY[3, 2, 1]`, `a > ALL (ARRAY[3,2,1])`},
+		{`3 > ALL ARRAY[3, 2, a]`, `3 > ALL (ARRAY[3, 2, a])`},
+		{`3 > ANY (ARRAY[3, 2, a])`, `3 > ANY (ARRAY[3, 2, a])`},
+		{`3 > SOME (((ARRAY[3, 2, a])))`, `3 > SOME (ARRAY[3, 2, a])`},
 		{`NULL LIKE 'a'`, `NULL`},
 		{`NULL NOT LIKE 'a'`, `NULL`},
 		{`NULL ILIKE 'a'`, `NULL`},

diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go
@@ -562,9 +562,9 @@ func TestParse(t *testing.T) {
 		{`SELECT a FROM t WHERE a IN (b, c)`},
 		{`SELECT a FROM t WHERE a IN (SELECT a FROM t)`},
 		{`SELECT a FROM t WHERE a NOT IN (b, c)`},
-		{`SELECT a FROM t WHERE a = ANY ARRAY[b, c]`},
-		{`SELECT a FROM t WHERE a != SOME ARRAY[b, c]`},
-		{`SELECT a FROM t WHERE a LIKE ALL ARRAY[b, c]`},
+		{`SELECT a FROM t WHERE a = ANY (ARRAY[b, c])`},
+		{`SELECT a FROM t WHERE a != SOME (ARRAY[b, c])`},
+		{`SELECT a FROM t WHERE a LIKE ALL (ARRAY[b, c])`},
 		{`SELECT a FROM t WHERE a LIKE b`},
 		{`SELECT a FROM t WHERE a NOT LIKE b`},
 		{`SELECT a FROM t WHERE a ILIKE b`},
@@ -1212,6 +1212,19 @@ func TestParse2(t *testing.T) {
 			`CREATE TABLE a (b INT, FOREIGN KEY (b) REFERENCES other ON UPDATE SET DEFAULT ON DELETE RESTRICT)`,
 			`CREATE TABLE a (b INT, FOREIGN KEY (b) REFERENCES other ON DELETE RESTRICT ON UPDATE SET DEFAULT)`,
 		},
+		// See #19555. This is actually invalid SQL, but we permit it anyway.
+		{
+			`SELECT a FROM t WHERE a = ANY ARRAY[b, c]`,
+			`SELECT a FROM t WHERE a = ANY (ARRAY[b, c])`,
+		},
+		{
+			`SELECT a FROM t WHERE a != SOME ARRAY[b, c]`,
+			`SELECT a FROM t WHERE a != SOME (ARRAY[b, c])`,
+		},
+		{
+			`SELECT a FROM t WHERE a LIKE ALL ARRAY[b, c]`,
+			`SELECT a FROM t WHERE a LIKE ALL (ARRAY[b, c])`,
+		},
 	}
 	for _, d := range testData {
 		stmts, err := Parse(d.sql)

diff --git a/pkg/sql/parser/type_check_test.go b/pkg/sql/parser/type_check_test.go
@@ -81,14 +81,14 @@ func TestTypeCheck(t *testing.T) {
 		{`ARRAY['a', 'b', 'c']`, `ARRAY['a':::STRING, 'b':::STRING, 'c':::STRING]`},
 		{`ARRAY[1.5, 2.5, 3.5]`, `ARRAY[1.5:::DECIMAL, 2.5:::DECIMAL, 3.5:::DECIMAL]`},
 		{`ARRAY[NULL]`, `ARRAY[NULL]`},
-		{`1 = ANY ARRAY[1.5, 2.5, 3.5]`, `1:::DECIMAL = ANY ARRAY[1.5:::DECIMAL, 2.5:::DECIMAL, 3.5:::DECIMAL]`},
-		{`true = SOME (ARRAY[true, false])`, `true = SOME ARRAY[true, false]`},
-		{`1.3 = ALL ARRAY[1, 2, 3]`, `1.3:::DECIMAL = ALL ARRAY[1:::DECIMAL, 2:::DECIMAL, 3:::DECIMAL]`},
-		{`1.3 = ALL ((ARRAY[]))`, `1.3:::DECIMAL = ALL ARRAY[]`},
+		{`1 = ANY ARRAY[1.5, 2.5, 3.5]`, `1:::DECIMAL = ANY (ARRAY[1.5:::DECIMAL, 2.5:::DECIMAL, 3.5:::DECIMAL])`},
+		{`true = SOME (ARRAY[true, false])`, `true = SOME (ARRAY[true, false])`},
+		{`1.3 = ALL ARRAY[1, 2, 3]`, `1.3:::DECIMAL = ALL (ARRAY[1:::DECIMAL, 2:::DECIMAL, 3:::DECIMAL])`},
+		{`1.3 = ALL ((ARRAY[]))`, `1.3:::DECIMAL = ALL (ARRAY[])`},
 		{`NULL = ALL ARRAY[1.5, 2.5, 3.5]`, `NULL`},
 		{`NULL = ALL ARRAY[NULL, NULL]`, `NULL`},
 		{`1 = ALL NULL`, `NULL`},
-		{`'a' = ALL CURRENT_SCHEMAS(true)`, `'a':::STRING = ALL current_schemas(true)`},
+		{`'a' = ALL CURRENT_SCHEMAS(true)`, `'a':::STRING = ALL (current_schemas(true))`},
 		{`NULL = ALL CURRENT_SCHEMAS(true)`, `NULL`},
 
 		{`INTERVAL '1'`, `'1s':::INTERVAL`},