Parentheses are incorrectly removed for type arrays in default expressions

[the-ericwang35]

Describe the problem

The way we currently serialize type arrays in default expressions removes parentheses in the expression. As a result, the expressions becomes invalid and results in an error.

To Reproduce

CREATE TYPE default_abc AS ENUM('a', 'b', 'c');
CREATE TABLE t (i STRING DEFAULT ('{a}'::_default_abc)[1]::STRING);

ERROR: at or near ":": syntax error
SQLSTATE: 42601
DETAIL: source SQL:
SET ROW ('{a}':::STRING::@100060[1:::INT8]::STRING)
                                  ^
HINT: try \h SET SESSION

Expected behavior
This syntax should be valid and not result in an error.
The expression should instead be serialized as ('{a}':::STRING::@100060)[1:::INT8]::STRING (notice the parentheses around '{a}':::STRING::@100060, which are currently being dropped).

[ajwerner]

Eric points out that the problem is that we drop all parents in type checking:

I think it happens here, during the type checking of ParenExpr:

cockroach/pkg/sql/sem/tree/type_check.go

Lines 1236 to 1247 in 9110a1b

	// TypeCheck implements the Expr interface.
	func (expr *ParenExpr) TypeCheck(
	ctx context.Context, semaCtx *SemaContext, desired *types.T,
	) (TypedExpr, error) {
	exprTyped, err := expr.Expr.TypeCheck(ctx, semaCtx, desired)
	if err != nil {
	return nil, err
	}
	// Parentheses are semantically unimportant and can be removed/replaced
	// with its nested expression in our plan. This makes type checking cleaner.
	return exprTyped, nil
	}

It basically just drops the parentheses and only returns the inner expr. There’s even a comment that says Parentheses are semantically unimportant and can be removed/replace

git blame says that it used to return the entire expression (including the parentheses if i’m not mistaken)
7a02e3a#diff-b2a517db9d4d185d9e0258c%5B%E2%80%A6%5D33c9277cdc6ba71ebbd95071L606

Unfortunately this seems too simplistic. It seems like any [...] tokens after a type get interpreted as part of the type. This is true in postgres too:

postgres=# CREATE TYPE typ AS ENUM ('a');
CREATE TYPE
postgres=# SELECT '{a}'::VARCHAR::_typ[1]::VARCHAR;
ERROR:  type "_typ[]" does not exist
postgres=# SELECT ('{a}'::VARCHAR::_typ)[1]::VARCHAR;
 varchar 
---------
 a

In short, it does seem like these parens are meaningful in this case. I wonder if there are other cases we haven't thought of? For now, one thing we can do is look and see if the outermost expression is a cast and retain the parens.

[ajwerner]

Another option is to let the ParensExpr remove itself but then inject parens in the case where you've got a cast expr in an indirection expr then you need to wrap it in a ParensExpr