sql: fix serialization of datums and prefix casts

This change addresses a few related problems (described in more detail
in #12532):
 - ambiguous datum types after serialization
 - invalid prefix casts after expression processing (like `INTERVAL INTERVAL `1.5s`').

Changes:

 - a new FmtFlag adds type annotations to datums and placeholders. This ensures
   that expressions that contain `DDInterval`, `DDate` can be formatted and
   re-parsed correctly. This replaces ad-hoc type prepending like
   `INTERVAL '..'` or `DATE '..'`.

 - `CastExpr.TypeCheck` now returns the underlying type-checked expression
   directly when it is a Datum that we know for sure is of identical type.

 - `castPrefix` (renamed to `castPrepend`) now only functions if the
   underlying expression is a string literal (preventing incorrect expressions
   like `DECIMAL '1.0':::STRING`). `castPrefixParens` is removed (it
   was incorrect).

 - `AnnotateTypeExpr.TypeCheck` now returns the underlying type-checked
   expression rather than changing the `AnnotateTypeExpr` in-place and
   returning it (which could result in double annotations with the new
   `FmtFlag`). `AnnotateTypeExpr` is no longer a `TypedExpr` so it is never part
   of a type-checked expression (which makes sense).

 - type checking tests are extended to verify the serialized type-checked
   expression, which verifies both serialization and type resolution.

Note that changing the prepend syntax to a `AnnotateTypeExpr` was considered
but that doesn't work for things like `DECIMAL '2.0'`.

Fixes #12532. Helps with #12611.

Author: RaduBerinde

pkg/sql/alter_table.go

@@ -487,7 +487,7 @@ func applyColumnMutation(
 			); err != nil {
 				return err
 			}
-			s := t.Default.String()
+			s := parser.Serialize(t.Default)
 			col.DefaultExpr = &s
 		}
 

pkg/sql/create.go

@@ -383,17 +383,21 @@ func (p *planner) CreateView(n *parser.CreateView) (planNode, error) {
 
 	var queryBuf bytes.Buffer
 	var fmtErr error
-	n.AsSource.Format(&queryBuf, parser.FmtNormalizeTableNames(
-		parser.FmtSimple,
-		func(t *parser.NormalizableTableName) *parser.TableName {
-			tn, err := p.QualifyWithDatabase(t)
-			if err != nil {
-				log.Warningf(p.ctx(), "failed to qualify table name %q with database name: %v", t, err)
-				fmtErr = err
-				return nil
-			}
-			return tn
-		}))
+	n.AsSource.Format(
+		&queryBuf,
+		parser.FmtNormalizeTableNames(
+			parser.FmtParsable,
+			func(t *parser.NormalizableTableName) *parser.TableName {
+				tn, err := p.QualifyWithDatabase(t)
+				if err != nil {
+					log.Warningf(p.ctx(), "failed to qualify table name %q with database name: %v", t, err)
+					fmtErr = err
+					return nil
+				}
+				return tn
+			},
+		),
+	)
 	if fmtErr != nil {
 		return nil, fmtErr
 	}
@@ -1471,7 +1475,7 @@ func makeCheckConstraint(
 			inuseNames[name] = struct{}{}
 		}
 	}
-	return &sqlbase.TableDescriptor_CheckConstraint{Expr: d.Expr.String(), Name: name}, nil
+	return &sqlbase.TableDescriptor_CheckConstraint{Expr: parser.Serialize(d.Expr), Name: name}, nil
 }
 
 // CreateTestTableDescriptor converts a SQL string to a table for test purposes.

pkg/sql/distsql_physical_planner.go

@@ -364,7 +364,7 @@ func mergePlans(
 // exprFmtFlagsBase are FmtFlags used for serializing expressions; a proper
 // IndexedVar formatting function needs to be added on.
 var exprFmtFlagsBase = parser.FmtStarDatumFormat(
-	parser.FmtSimple,
+	parser.FmtParsable,
 	func(buf *bytes.Buffer, _ parser.FmtFlags) {
 		fmt.Fprintf(buf, "0")
 	},

pkg/sql/parser/datum.go

@@ -56,17 +56,25 @@ var (
 // Datum represents a SQL value.
 type Datum interface {
 	TypedExpr
+
+	// AmbiguousFormat indicates whether the result of formatting this Datum can
+	// be interpreted into more than one type. Used with
+	// fmtFlags.disambiguateDatumTypes.
+	AmbiguousFormat() bool
+
 	// Compare returns -1 if the receiver is less than other, 0 if receiver is
 	// equal to other and +1 if receiver is greater than other.
 	// TODO(nvanbenschoten) Should we look into merging this with cmpOps?
 	Compare(other Datum) int
+
 	// Prev returns the previous datum and true, if one exists, or nil
 	// and false.  The previous datum satisfied the following
 	// definition: if the receiver is "b" and the returned datum is "a",
 	// then "a < b" and no other datum will compare such that "a < c <
 	// b".
 	// The return value is undefined if `IsMin()` returns true.
 	Prev() (Datum, bool)
+
 	// IsMin returns true if the datum is equal to the minimum value the datum
 	// type can hold.
 	IsMin() bool
@@ -78,6 +86,7 @@ type Datum interface {
 	// b".
 	// The return value is undefined if `IsMax()` returns true.
 	Next() (Datum, bool)
+
 	// IsMax returns true if the datum is equal to the maximum value the datum
 	// type can hold.
 	IsMax() bool
@@ -202,6 +211,9 @@ func (d *DBool) max() (Datum, bool) {
 	return DBoolTrue, true
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DBool) AmbiguousFormat() bool { return false }
+
 // Format implements the NodeFormatter interface.
 func (d *DBool) Format(buf *bytes.Buffer, f FmtFlags) {
 	buf.WriteString(strconv.FormatBool(bool(*d)))
@@ -291,6 +303,9 @@ func (d *DInt) min() (Datum, bool) {
 	return dMinInt, true
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DInt) AmbiguousFormat() bool { return true }
+
 // Format implements the NodeFormatter interface.
 func (d *DInt) Format(buf *bytes.Buffer, f FmtFlags) {
 	buf.WriteString(strconv.FormatInt(int64(*d), 10))
@@ -391,6 +406,9 @@ func (d *DFloat) min() (Datum, bool) {
 	return dMinFloat, true
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DFloat) AmbiguousFormat() bool { return true }
+
 // Format implements the NodeFormatter interface.
 func (d *DFloat) Format(buf *bytes.Buffer, f FmtFlags) {
 	fl := float64(*d)
@@ -474,6 +492,9 @@ func (d *DDecimal) min() (Datum, bool) {
 	return nil, false
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DDecimal) AmbiguousFormat() bool { return true }
+
 // Format implements the NodeFormatter interface.
 func (d *DDecimal) Format(buf *bytes.Buffer, f FmtFlags) {
 	buf.WriteString(d.Dec.String())
@@ -551,6 +572,9 @@ func (d *DString) max() (Datum, bool) {
 	return nil, false
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DString) AmbiguousFormat() bool { return true }
+
 // Format implements the NodeFormatter interface.
 func (d *DString) Format(buf *bytes.Buffer, f FmtFlags) {
 	encodeSQLStringWithFlags(buf, string(*d), f)
@@ -607,6 +631,9 @@ func NewDCollatedString(contents string, locale string, env *CollationEnvironmen
 	return &d
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DCollatedString) AmbiguousFormat() bool { return false }
+
 // Format implements the NodeFormatter interface.
 func (d *DCollatedString) Format(buf *bytes.Buffer, f FmtFlags) {
 	encodeSQLString(buf, d.Contents)
@@ -731,6 +758,9 @@ func (d *DBytes) max() (Datum, bool) {
 	return nil, false
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DBytes) AmbiguousFormat() bool { return false }
+
 // Format implements the NodeFormatter interface.
 func (d *DBytes) Format(buf *bytes.Buffer, f FmtFlags) {
 	encodeSQLBytes(buf, string(*d))
@@ -848,6 +878,9 @@ func (d *DDate) min() (Datum, bool) {
 	return nil, false
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DDate) AmbiguousFormat() bool { return true }
+
 // Format implements the NodeFormatter interface.
 func (d *DDate) Format(buf *bytes.Buffer, f FmtFlags) {
 	if !f.bareStrings {
@@ -1003,6 +1036,9 @@ func (d *DTimestamp) max() (Datum, bool) {
 	return nil, false
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DTimestamp) AmbiguousFormat() bool { return true }
+
 // Format implements the NodeFormatter interface.
 func (d *DTimestamp) Format(buf *bytes.Buffer, f FmtFlags) {
 	if !f.bareStrings {
@@ -1107,6 +1143,9 @@ func (d *DTimestampTZ) max() (Datum, bool) {
 	return nil, false
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DTimestampTZ) AmbiguousFormat() bool { return true }
+
 // Format implements the NodeFormatter interface.
 func (d *DTimestampTZ) Format(buf *bytes.Buffer, f FmtFlags) {
 	if !f.bareStrings {
@@ -1321,11 +1360,18 @@ func (d *DInterval) ValueAsString() string {
 	return (time.Duration(d.Duration.Nanos) * time.Nanosecond).String()
 }
 
-// Format implements the NodeFormatter interface. Example: "INTERVAL `1h2m`".
+// AmbiguousFormat implements the Datum interface.
+func (*DInterval) AmbiguousFormat() bool { return true }
+
+// Format implements the NodeFormatter interface.
 func (d *DInterval) Format(buf *bytes.Buffer, f FmtFlags) {
-	buf.WriteString("INTERVAL '")
+	if !f.bareStrings {
+		buf.WriteByte('\'')
+	}
 	buf.WriteString(d.ValueAsString())
-	buf.WriteByte('\'')
+	if !f.bareStrings {
+		buf.WriteByte('\'')
+	}
 }
 
 // Size implements the Datum interface.
@@ -1482,6 +1528,9 @@ func (d *DTuple) IsMin() bool {
 	return true
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DTuple) AmbiguousFormat() bool { return false }
+
 // Format implements the NodeFormatter interface.
 func (d *DTuple) Format(buf *bytes.Buffer, f FmtFlags) {
 	buf.WriteByte('(')
@@ -1602,6 +1651,9 @@ func (dNull) min() (Datum, bool) {
 	return DNull, true
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (dNull) AmbiguousFormat() bool { return false }
+
 // Format implements the NodeFormatter interface.
 func (dNull) Format(buf *bytes.Buffer, f FmtFlags) {
 	buf.WriteString("NULL")
@@ -1691,6 +1743,9 @@ func (d *DArray) IsMin() bool {
 	return d.Len() == 0
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DArray) AmbiguousFormat() bool { return false }
+
 // Format implements the NodeFormatter interface.
 func (d *DArray) Format(buf *bytes.Buffer, f FmtFlags) {
 	buf.WriteByte('{')
@@ -1760,6 +1815,9 @@ type DTable struct {
 	ValueGenerator
 }
 
+// AmbiguousFormat implements the Datum interface.
+func (*DTable) AmbiguousFormat() bool { return false }
+
 // Format implements the NodeFormatter interface.
 func (t *DTable) Format(buf *bytes.Buffer, _ FmtFlags) {
 	buf.WriteString("<generated>")

pkg/sql/parser/datum_test.go

@@ -93,15 +93,15 @@ func TestDatumOrdering(t *testing.T) {
 
 		// Intervals
 		{`'1 day':::interval`, noPrev, noNext,
-			`INTERVAL '-9223372036854775808m-9223372036854775808d-2562047h47m16.854775808s'`,
-			`INTERVAL '9223372036854775807m9223372036854775807d2562047h47m16.854775807s'`},
+			`'-9223372036854775808m-9223372036854775808d-2562047h47m16.854775808s'`,
+			`'9223372036854775807m9223372036854775807d2562047h47m16.854775807s'`},
 		// Max interval: we use Postgres syntax, because Go doesn't accept
 		// months/days and ISO8601 doesn't accept nanoseconds.
 		{`'9223372036854775807 months 9223372036854775807 days ` +
 			`2562047 hours 47 minutes 16 seconds 854775807 nanoseconds':::interval`,
 			noPrev, valIsMax,
-			`INTERVAL '-9223372036854775808m-9223372036854775808d-2562047h47m16.854775808s'`,
-			`INTERVAL '9223372036854775807m9223372036854775807d2562047h47m16.854775807s'`},
+			`'-9223372036854775808m-9223372036854775808d-2562047h47m16.854775808s'`,
+			`'9223372036854775807m9223372036854775807d2562047h47m16.854775807s'`},
 		// It's hard to generate a minimum interval! We can't use a
 		// negative value inside the string constant, because that's not
 		// allowed in a Postgres duration specification. We can't use the
@@ -113,8 +113,8 @@ func TestDatumOrdering(t *testing.T) {
 			`2562047 hours 47 minutes 16 seconds':::interval` +
 			`-'1 month 1 day 854775808 nanoseconds':::interval`,
 			valIsMin, noNext,
-			`INTERVAL '-9223372036854775808m-9223372036854775808d-2562047h47m16.854775808s'`,
-			`INTERVAL '9223372036854775807m9223372036854775807d2562047h47m16.854775807s'`},
+			`'-9223372036854775808m-9223372036854775808d-2562047h47m16.854775808s'`,
+			`'9223372036854775807m9223372036854775807d2562047h47m16.854775807s'`},
 
 		// NULL
 		{`NULL`, valIsMin, valIsMax, `NULL`, `NULL`},

pkg/sql/parser/eval.go

@@ -2248,11 +2248,6 @@ func (expr *IndirectionExpr) Eval(ctx *EvalContext) (Datum, error) {
 	return arr.Array[subscriptIdx-1], nil
 }
 
-// Eval implements the TypedExpr interface.
-func (expr *AnnotateTypeExpr) Eval(ctx *EvalContext) (Datum, error) {
-	return expr.Expr.(TypedExpr).Eval(ctx)
-}
-
 // Eval implements the TypedExpr interface.
 func (expr *CollateExpr) Eval(ctx *EvalContext) (Datum, error) {
 	d, err := expr.Expr.(TypedExpr).Eval(ctx)