From 5a729eed13b2abe492561470548f44f6ffd5e9c3 Mon Sep 17 00:00:00 2001 From: Jordan Lewis Date: Thu, 26 Oct 2017 02:13:49 -0400 Subject: [PATCH] sql: disambiguate ANY/ALL/SOME exprs in formatting Previously, ANY/ALL/SOME expressions would be formatted without parentheses around their antecedents. This could lead to invalid input. For example, the following query doesn't parse properly since the type annotation binds to the comparison expression, not the array: ``` SELECT 1 = ANY ARRAY[1]:::INT[] ``` This patch adds the disambiguating enclosing parenthesis. --- pkg/sql/index_selection_test.go | 2 +- pkg/sql/parser/expr.go | 4 +++- pkg/sql/parser/normalize_test.go | 8 ++++---- pkg/sql/parser/parse_test.go | 19 ++++++++++++++++--- pkg/sql/parser/type_check_test.go | 10 +++++----- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/pkg/sql/index_selection_test.go b/pkg/sql/index_selection_test.go index ec4818691f0b..6b63acbc91d7 100644 --- 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 index f3d75b0c5053..3a18bdf8a96f 100644 --- 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 index 607cfc32a8f5..a7a97c0d9c3a 100644 --- 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 index 6ad05e09a88a..2c0441a4dae5 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -560,9 +560,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`}, @@ -1210,6 +1210,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 index 3076407af659..4ef770547917 100644 --- 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`},