sql: remove type modifiers from placeholder types

The width and precision of placeholder values are not known during
PREPARE, therefore we remove any type modifiers from placeholder types
during type checking so that a value of any width will fit within the
palceholder type.

Note that this change is similar to the changes made in #70722 to
placeholder type checking that were later reverted in #72793. In #70722
the type OIDs of placeholders could be altered, e.g., a placeholder
originally with type `INT2` would be converted to an `INT`. In this
commit, type OIDs of placeholders are not changing, only type modifiers
are.

This commit should allow some logic added in #72793 to be simplified or
removed entirely.

Release note: None

pkg/sql/information_schema.go

@@ -36,6 +36,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/vtable"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil/pgdate"
 	"github.com/cockroachdb/errors"
+	"github.com/lib/pq/oid"
 	"golang.org/x/text/collate"
 )
 
@@ -617,6 +618,11 @@ https://www.postgresql.org/docs/9.5/infoschema-enabled-roles.html`,
 // string, or if the string's length is not bounded.
 func characterMaximumLength(colType *types.T) tree.Datum {
 	return dIntFnOrNull(func() (int32, bool) {
+		// "char" columns have a width of 1, but should report a NULL maximum
+		// character length.
+		if colType.Oid() == oid.T_char {
+			return 0, false
+		}
 		switch colType.Family() {
 		case types.StringFamily, types.CollatedStringFamily, types.BitFamily:
 			if colType.Width() > 0 {
@@ -633,6 +639,11 @@ func characterMaximumLength(colType *types.T) tree.Datum {
 // string's length is not bounded.
 func characterOctetLength(colType *types.T) tree.Datum {
 	return dIntFnOrNull(func() (int32, bool) {
+		// "char" columns have a width of 1, but should report a NULL octet
+		// length.
+		if colType.Oid() == oid.T_char {
+			return 0, false
+		}
 		switch colType.Family() {
 		case types.StringFamily, types.CollatedStringFamily:
 			if colType.Width() > 0 {

pkg/sql/logictest/testdata/logic_test/cast

@@ -147,6 +147,12 @@ INSERT INTO assn_cast(t) VALUES ('1970-01-01'::timestamptz)
 statement ok
 INSERT INTO assn_cast(d) VALUES (11.22), (88.99)
 
+statement ok
+INSERT INTO assn_cast(d) VALUES (22.33::DECIMAL(10, 0)), (99.11::DECIMAL(10, 2))
+
+statement ok
+INSERT INTO assn_cast(d) VALUES (33.11::DECIMAL(10, 0)), (44.44::DECIMAL(10, 0))
+
 statement ok
 PREPARE insert_d AS INSERT INTO assn_cast(d) VALUES ($1)
 
@@ -164,9 +170,19 @@ SELECT d FROM assn_cast WHERE d IS NOT NULL
 ----
 11
 89
+22
+99
+33
+44
 123
 68
 
+statement ok
+INSERT INTO assn_cast(a) VALUES (ARRAY[2.88, NULL, 15])
+
+statement ok
+INSERT INTO assn_cast(a) VALUES (ARRAY[3.99, NULL, 16]::DECIMAL(10, 2)[])
+
 statement ok
 INSERT INTO assn_cast(a) VALUES (ARRAY[5.55, 6.66::DECIMAL(10, 2)])
 
@@ -179,6 +195,8 @@ EXECUTE insert_a(ARRAY[7.77, 8.88::DECIMAL(10, 2)])
 query T rowsort
 SELECT a FROM assn_cast WHERE a IS NOT NULL
 ----
+{3,NULL,15}
+{4,NULL,16}
 {6,7}
 {8,9}
 

pkg/sql/logictest/testdata/logic_test/information_schema

@@ -2373,7 +2373,7 @@ char_len    dc           1                         4
 char_len    ec           12                        48
 char_len    dv           NULL                      NULL
 char_len    ev           12                        48
-char_len    dq           1                         4
+char_len    dq           NULL                      NULL
 char_len    f            NULL                      NULL
 char_len    g            1                         NULL
 char_len    h            12                        NULL

pkg/sql/opt/norm/testdata/rules/fold_constants

@@ -926,7 +926,7 @@ insert assn_cast
       ├── fd: ()-->(9-13)
       └── tuple
            ├── assignment-cast: CHAR
-           │    └── $1
+           │    └── $1::CHAR
            ├── CAST(NULL AS "char")
            ├── CAST(NULL AS INT8)
            ├── CAST(NULL AS STRING)

pkg/sql/sem/tree/type_check.go

@@ -1599,9 +1599,15 @@ func (expr *Placeholder) TypeCheck(
 	// when there are no available values for the placeholders yet, because
 	// during Execute all placeholders are replaced from the AST before type
 	// checking.
+	//
+	// The width of a placeholder value is not known during Prepare, so we
+	// remove type modifiers from the desired type so that a value of any width
+	// will fit within the placeholder type.
+	desired = desired.WithoutTypeModifiers()
 	if typ, ok, err := semaCtx.Placeholders.Type(expr.Idx); err != nil {
 		return expr, err
 	} else if ok {
+		typ = typ.WithoutTypeModifiers()
 		if !desired.Equivalent(typ) {
 			// This indicates there's a conflict between what the type system thinks
 			// the type for this position should be, and the actual type of the

pkg/sql/sem/tree/type_check_test.go

@@ -123,8 +123,8 @@ func TestTypeCheck(t *testing.T) {
 		{`1 + $1`, `1:::INT8 + $1:::INT8`},
 		{`1:::DECIMAL + $1`, `1:::DECIMAL + $1:::DECIMAL`},
 		{`$1:::INT8`, `$1:::INT8`},
-		{`2::DECIMAL(10,2) + $1`, `2:::DECIMAL::DECIMAL(10,2) + $1:::DECIMAL(10,2)`},
-		{`2::DECIMAL(10,0) + $1`, `2:::DECIMAL::DECIMAL(10) + $1:::DECIMAL(10)`},
+		{`2::DECIMAL(10,2) + $1`, `2:::DECIMAL::DECIMAL(10,2) + $1:::DECIMAL`},
+		{`2::DECIMAL(10,0) + $1`, `2:::DECIMAL::DECIMAL(10) + $1:::DECIMAL`},
 
 		// Tuples with labels
 		{`(ROW (1) AS a)`, `((1:::INT8,) AS a)`},

pkg/sql/types/types.go

@@ -320,7 +320,7 @@ var (
 	//
 	// See https://www.postgresql.org/docs/9.1/static/datatype-character.html
 	QChar = &T{InternalType: InternalType{
-		Family: StringFamily, Oid: oid.T_char, Width: 1, Locale: &emptyLocale}}
+		Family: StringFamily, Width: 1, Oid: oid.T_char, Locale: &emptyLocale}}
 
 	// Name is a type-alias for String with a different OID (T_name). It is
 	// reported as NAME in SHOW CREATE and "name" in introspection for
@@ -614,16 +614,22 @@ var (
 	typeBit = &T{InternalType: InternalType{
 		Family: BitFamily, Oid: oid.T_bit, Locale: &emptyLocale}}
 
-	// typeBpChar is the "standard SQL" string type of fixed length, where "bp"
-	// stands for "blank padded". It is not exported to avoid confusion with
-	// QChar, as well as confusion over its default width.
+	// typeBpChar is a CHAR type with an unspecified width. "bp" stands for
+	// "blank padded". It is not exported to avoid confusion with QChar, as well
+	// as confusion over CHAR's default width of 1.
 	//
 	// It is reported as CHAR in SHOW CREATE and "character" in introspection for
 	// compatibility with PostgreSQL.
-	//
-	// Its default maximum with is 1. It always has a maximum width.
 	typeBpChar = &T{InternalType: InternalType{
 		Family: StringFamily, Oid: oid.T_bpchar, Locale: &emptyLocale}}
+
+	// typeQChar is a "char" type with an unspecified width. It is not exported
+	// to avoid confusion with QChar. The "char" type should always have a width
+	// of one. A "char" type with an unspecified width is only used when the
+	// length of a "char" value cannot be determined, for example a placeholder
+	// typed as a "char" should have an unspecified width.
+	typeQChar = &T{InternalType: InternalType{
+		Family: StringFamily, Oid: oid.T_char, Locale: &emptyLocale}}
 )
 
 const (
@@ -821,10 +827,13 @@ func MakeVarChar(width int32) *T {
 }
 
 // MakeChar constructs a new instance of the CHAR type (oid = T_bpchar) having
-// the given max number of characters.
+// the given max # characters (0 = unspecified number).
 func MakeChar(width int32) *T {
-	if width <= 0 {
-		panic(errors.AssertionFailedf("width for type char must be at least 1"))
+	if width == 0 {
+		return typeBpChar
+	}
+	if width < 0 {
+		panic(errors.AssertionFailedf("width %d cannot be negative", width))
 	}
 	return &T{InternalType: InternalType{
 		Family: StringFamily, Oid: oid.T_bpchar, Width: width, Locale: &emptyLocale}}
@@ -1231,6 +1240,86 @@ func (t *T) TypeModifier() int32 {
 	return int32(-1)
 }
 
+// WithoutTypeModifiers returns a copy of the given type with the type modifiers
+// reset, if the type has modifiers. The returned type has arbitrary width and
+// precision, or for some types, like timestamps, the maximum allowed width and
+// precision. If the given type already has no type modifiers, it is returned
+// unchanged and the function does not allocate a new type.
+func (t *T) WithoutTypeModifiers() *T {
+	switch t.Family() {
+	case ArrayFamily:
+		// Remove type modifiers of the array content type.
+		newContents := t.ArrayContents().WithoutTypeModifiers()
+		if newContents == t.ArrayContents() {
+			return t
+		}
+		return MakeArray(newContents)
+	case TupleFamily:
+		// Remove type modifiers for each of the tuple content types.
+		oldContents := t.TupleContents()
+		newContents := make([]*T, len(oldContents))
+		changed := false
+		for i := range newContents {
+			newContents[i] = oldContents[i].WithoutTypeModifiers()
+			if newContents[i] != oldContents[i] {
+				changed = true
+			}
+		}
+		if !changed {
+			return t
+		}
+		return MakeTuple(newContents)
+	case EnumFamily:
+		// Enums have no type modifiers.
+		return t
+	}
+
+	switch t.Oid() {
+	case oid.T_bit:
+		return MakeBit(0)
+	case oid.T_bpchar, oid.T_char, oid.T_text, oid.T_varchar:
+		// For string-like types, we copy the type and set the width to 0 rather
+		// than returning typeBpChar, typeQChar, VarChar, or String so that
+		// we retain the locale value if the type is collated.
+		newT := *t
+		newT.InternalType.Width = 0
+		return &newT
+	case oid.T_interval:
+		return Interval
+	case oid.T_numeric:
+		return Decimal
+	case oid.T_time:
+		return Time
+	case oid.T_timestamp:
+		return Timestamp
+	case oid.T_timestamptz:
+		return TimestampTZ
+	case oid.T_timetz:
+		return TimeTZ
+	case oid.T_varbit:
+		return VarBit
+	case oid.T_anyelement,
+		oid.T_bool,
+		oid.T_bytea,
+		oid.T_date,
+		oidext.T_box2d,
+		oid.T_float4, oid.T_float8,
+		oidext.T_geography, oidext.T_geometry,
+		oid.T_inet,
+		oid.T_int2, oid.T_int4, oid.T_int8,
+		oid.T_jsonb,
+		oid.T_name,
+		oid.T_oid,
+		oid.T_regclass, oid.T_regnamespace, oid.T_regproc, oid.T_regprocedure, oid.T_regrole, oid.T_regtype,
+		oid.T_unknown,
+		oid.T_uuid,
+		oid.T_void:
+		return t
+	default:
+		panic(errors.AssertionFailedf("unexpected OID: %d", t.Oid()))
+	}
+}
+
 // Scale is an alias method for Width, used for clarity for types in
 // DecimalFamily.
 func (t *T) Scale() int32 {

pkg/sql/types/types_test.go

@@ -979,3 +979,55 @@ func TestSQLStandardName(t *testing.T) {
 		})
 	}
 }
+
+func TestWithoutTypeModifiers(t *testing.T) {
+	testCases := []struct {
+		t        *T
+		expected *T
+	}{
+		// Types with modifiers.
+		{MakeBit(2), typeBit},
+		{MakeVarBit(2), VarBit},
+		{MakeString(2), String},
+		{MakeVarChar(2), VarChar},
+		{MakeChar(2), typeBpChar},
+		{QChar, typeQChar},
+		{MakeCollatedString(MakeString(2), "en"), MakeCollatedString(String, "en")},
+		{MakeCollatedString(MakeVarChar(2), "en"), MakeCollatedString(VarChar, "en")},
+		{MakeCollatedString(MakeChar(2), "en"), MakeCollatedString(typeBpChar, "en")},
+		{MakeCollatedString(QChar, "en"), MakeCollatedString(typeQChar, "en")},
+		{MakeDecimal(5, 1), Decimal},
+		{MakeTime(2), Time},
+		{MakeTimeTZ(2), TimeTZ},
+		{MakeTimestamp(2), Timestamp},
+		{MakeTimestampTZ(2), TimestampTZ},
+		{MakeInterval(IntervalTypeMetadata{Precision: 3, PrecisionIsSet: true}),
+			Interval},
+		{MakeArray(MakeDecimal(5, 1)), DecimalArray},
+		{MakeTuple([]*T{MakeString(2), Time, MakeDecimal(5, 1)}),
+			MakeTuple([]*T{String, Time, Decimal})},
+
+		// Types without modifiers.
+		{Bool, Bool},
+		{Bytes, Bytes},
+		{MakeCollatedString(Name, "en"), MakeCollatedString(Name, "en")},
+		{Date, Date},
+		{Float, Float},
+		{Float4, Float4},
+		{Geography, Geography},
+		{Geometry, Geometry},
+		{INet, INet},
+		{Int, Int},
+		{Int4, Int4},
+		{Int2, Int2},
+		{Jsonb, Jsonb},
+		{Name, Name},
+		{Uuid, Uuid},
+	}
+
+	for _, tc := range testCases {
+		if actual := tc.t.WithoutTypeModifiers(); !actual.Identical(tc.expected) {
+			t.Errorf("expected <%v>, got <%v>", tc.expected.DebugString(), actual.DebugString())
+		}
+	}
+}