sql: format arrays in pg_catalog to match Postgres

[rafiss]

Postgres uses the curly brace syntax to display array constants that are
in pg_catalog, like those in default expressions.

CockroachDB stores arrays in descriptors as tree.Array exprs, so in
order to use the curly brace syntax, now we check if the array has any
variables or functions, and if not, we turn the Array expr into a
DArray. The formatting logic for DArrays also is updated to use single
quotes as appropriate, when formatting for pg_catalog.

fixes #55320

Release note (sql change): Arrays in pg_catalog tables now are displyed
in a format that matches PostgreSQL.

[cockroach-teamcity]

This change is 

[arulajmani]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani, @jordanlewis, and @rafiss)

---

pkg/sql/catalog/schemaexpr/expr.go, line 202 at r1 (raw file):

		sanitizedExpr, err := SanitizeVarFreeExpr(ctx, expr, typedExpr.ResolvedType(), "FORMAT", semaCtx,
			tree.VolatilityImmutable)
		if err == nil {

This err == nil could use some commentary. Better yet, are we expecting a particular type of error here in case we want to fall back to the typedExpr? If so, can we check for that and be more explicit?

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @jordanlewis)

---

pkg/sql/catalog/schemaexpr/expr.go, line 202 at r1 (raw file):

Previously, arulajmani (Arul Ajmani) wrote…

This err == nil could use some commentary. Better yet, are we expecting a particular type of error here in case we want to fall back to the typedExpr? If so, can we check for that and be more explicit?

i don't think we can check for a specific error. the error could be

pgerror.Newf(pgcode.Syntax,
			"variable sub-expressions are not allowed in %s", context)

or anything from the typechecker rejecting the volatility of the expression

agree on commenting!

[arulajmani]

Thanks for making the changes! modulo a couple of nits re: the comments for future us!

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani, @jordanlewis, and @rafiss)

---

pkg/sql/catalog/schemaexpr/expr.go, line 202 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i don't think we can check for a specific error. the error could be

pgerror.Newf(pgcode.Syntax,
			"variable sub-expressions are not allowed in %s", context)

or anything from the typechecker rejecting the volatility of the expression

agree on commenting!

nit: the comment about an empty EvalContext might be misplaced -- shouldn't it go above the d, err := sanitizedExpr.Eval(&tree.EvalContext()) line?

nit: Could you also add a nod to the fact that this is best effort and we can fall back on returning an expression if we can't convert it to a constant?

[rafiss]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @jordanlewis)

---

pkg/sql/catalog/schemaexpr/expr.go, line 202 at r1 (raw file):

Previously, arulajmani (Arul Ajmani) wrote…

nit: the comment about an empty EvalContext might be misplaced -- shouldn't it go above the d, err := sanitizedExpr.Eval(&tree.EvalContext()) line?

nit: Could you also add a nod to the fact that this is best effort and we can fall back on returning an expression if we can't convert it to a constant?

thanks -- i shouldn't have moved that comment. also adding the other one

[rafiss]

bors r=arulajmani

[craig]

Build succeeded:

• GitHub CI (Cockroach)