Skip to content

Commit

Permalink
Merge #63271
Browse files Browse the repository at this point in the history
63271: sql: wrap typed Array within IndirectionExpr in ParenExpr r=ajwerner,rafiss a=the-ericwang35

This patch adds a case that #63103 missed.

When we have something like `ARRAY['a'::typ, 'b'::typ]`,
we format it to `ARRAY['a'::typ, 'b'::typ]:::public.typ[]`.
This causes issues if used in an `IndirectionExpr`, such as
`ARRAY['a'::typ, 'b'::typ]:::public.typ[][1]`, as the `[1]`
is interpreted as part of the type.
This would result in errors when trying to use these
expressions in default expressions.
This patch checks for this specific case, and instead
formats it as `(ARRAY['a'::typ, 'b'::typ]:::public.typ[])[1]`.

Release note: None

Co-authored-by: Eric Wang <[email protected]>
  • Loading branch information
craig[bot] and the-ericwang35 committed Apr 8, 2021
2 parents f747927 + 3a0c88c commit 0c93de1
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 3 deletions.
22 changes: 22 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/enums
Original file line number Diff line number Diff line change
Expand Up @@ -1363,3 +1363,25 @@ query T
SELECT * FROM arr_t3
----
c

statement ok
CREATE TABLE arr_t4 (i typ DEFAULT ARRAY['a'::typ][1], j typ DEFAULT ARRAY['a'::typ, 'b'::typ, 'c'::typ][2])

statement ok
INSERT INTO arr_t4 VALUES (default, default)

query TT
SELECT * from arr_t4
----
a b

statement ok
CREATE TABLE arr_t5 (i typ[] default ARRAY['a'::typ, 'b'::typ])

statement ok
INSERT INTO arr_t5 VALUES (default)

query T
SELECT * from arr_t5
----
{a,b}
8 changes: 8 additions & 0 deletions pkg/sql/parser/testdata/parse/create_table
Original file line number Diff line number Diff line change
Expand Up @@ -2150,3 +2150,11 @@ CREATE TABLE arr_t (i INT8 DEFAULT ((('{' || '1') || '}')::INT8[])[1]) -- normal
CREATE TABLE arr_t (i INT8 DEFAULT (((((((((('{') || ('1'))) || ('}'))))::INT8[])))[(1)])) -- fully parenthetized
CREATE TABLE arr_t (i INT8 DEFAULT (((_ || _) || _)::INT8[])[_]) -- literals removed
CREATE TABLE _ (_ INT8 DEFAULT ((('{' || '1') || '}')::INT8[])[1]) -- identifiers removed

parse
CREATE TABLE arr_t (i INT8 DEFAULT (ARRAY[1, 2, 3]::int[])[2])
----
CREATE TABLE arr_t (i INT8 DEFAULT (ARRAY[1, 2, 3]::INT8[])[2]) -- normalized!
CREATE TABLE arr_t (i INT8 DEFAULT (((((ARRAY[(1), (2), (3)])::INT8[])))[(2)])) -- fully parenthetized
CREATE TABLE arr_t (i INT8 DEFAULT (ARRAY[_, _, __more1__]::INT8[])[_]) -- literals removed
CREATE TABLE _ (_ INT8 DEFAULT (ARRAY[1, 2, 3]::INT8[])[2]) -- identifiers removed
14 changes: 11 additions & 3 deletions pkg/sql/sem/tree/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -1624,10 +1624,18 @@ type IndirectionExpr struct {

// Format implements the NodeFormatter interface.
func (node *IndirectionExpr) Format(ctx *FmtCtx) {
// If the subExpr is a CastExpr, we need to wrap it in a ParenExpr,
// otherwise the indirection will get interpreted as part of the type.
// If the sub expression is a CastExpr or an Array that has a type,
// we need to wrap it in a ParenExpr, otherwise the indirection
// will get interpreted as part of the type.
// Ex. ('{a}'::_typ)[1] vs. '{a}'::_typ[1].
if _, isCast := node.Expr.(*CastExpr); isCast {
// Ex. (ARRAY['a'::typ]:::typ[])[1] vs. ARRAY['a'::typ]:::typ[][1].
var annotateArray bool
if arr, ok := node.Expr.(*Array); ctx.HasFlags(FmtParsable) && ok && arr.typ != nil {
if arr.typ.ArrayContents().Family() != types.UnknownFamily {
annotateArray = true
}
}
if _, isCast := node.Expr.(*CastExpr); isCast || annotateArray {
withParens := ParenExpr{Expr: node.Expr}
exprFmtWithParen(ctx, &withParens)
} else {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/tree/type_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ func TestTypeCheck(t *testing.T) {

{`(('{' || 'a' ||'}')::STRING[])[1]::STRING`, `((('{':::STRING || 'a':::STRING) || '}':::STRING)::STRING[])[1:::INT8]`},
{`(('{' || '1' ||'}')::INT[])[1]`, `((('{':::STRING || '1':::STRING) || '}':::STRING)::INT8[])[1:::INT8]`},
{`(ARRAY[1, 2, 3]::int[])[2]`, `(ARRAY[1:::INT8, 2:::INT8, 3:::INT8]:::INT8[])[2:::INT8]`},
}
ctx := context.Background()
for _, d := range testData {
Expand Down

0 comments on commit 0c93de1

Please sign in to comment.