From db595652856041b144446c2ce7537fccbcb8bcfb Mon Sep 17 00:00:00 2001
From: Raphael 'kena' Poss <knz@thaumogen.net>
Date: Wed, 15 Dec 2021 07:05:11 +0100
Subject: [PATCH 1/4] sql/opt/exec: output index/expiry in EXPLAIN
 SPLIT/RELOCATE statements

Release note (sql change): The output of `EXPLAIN ALTER INDEX/TABLE
... RELOCATE/SPLIT` now includes the target table/index name and, for
the SPLIT AT variants, the expiry timestamp.
---
 pkg/sql/opt/exec/execbuilder/testdata/ddl     |  2 ++
 pkg/sql/opt/exec/execbuilder/testdata/explain | 23 +++++++++++++++++++
 .../opt/exec/execbuilder/testdata/subquery    |  2 ++
 pkg/sql/opt/exec/explain/emit.go              | 21 +++++++++++++----
 pkg/sql/opt/exec/explain/testdata/gists       |  9 ++++++++
 5 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/pkg/sql/opt/exec/execbuilder/testdata/ddl b/pkg/sql/opt/exec/execbuilder/testdata/ddl
index 7e9511cc8324..c1bd125e5a47 100644
--- a/pkg/sql/opt/exec/execbuilder/testdata/ddl
+++ b/pkg/sql/opt/exec/execbuilder/testdata/ddl
@@ -101,6 +101,8 @@ vectorized: true
 • split
 │ columns: (key, pretty, split_enforced_until)
 │ estimated row count: 10 (missing stats)
+│ index: s@s_pkey
+│ expiry: CAST(NULL AS STRING)
 │
 └── • scan
       columns: (k1, k2)
diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain b/pkg/sql/opt/exec/execbuilder/testdata/explain
index 8e7141bfce5c..aa4fcd1fdfaf 100644
--- a/pkg/sql/opt/exec/execbuilder/testdata/explain
+++ b/pkg/sql/opt/exec/execbuilder/testdata/explain
@@ -170,6 +170,25 @@ vectorized: true
 • split
 │ columns: (key, pretty, split_enforced_until)
 │ estimated row count: 10 (missing stats)
+│ index: foo@foo_pkey
+│ expiry: CAST(NULL AS STRING)
+│
+└── • values
+      columns: (column1)
+      size: 1 column, 1 row
+      row 0, expr 0: 42
+
+query T
+EXPLAIN (VERBOSE) ALTER TABLE foo SPLIT AT VALUES (42) WITH EXPIRATION '2200-01-01 00:00:00.0'
+----
+distribution: local
+vectorized: true
+·
+• split
+│ columns: (key, pretty, split_enforced_until)
+│ estimated row count: 10 (missing stats)
+│ index: foo@foo_pkey
+│ expiry: '2200-01-01 00:00:00.0'
 │
 └── • values
       columns: (column1)
@@ -183,6 +202,7 @@ distribution: local
 vectorized: true
 ·
 • relocate table
+│ index: foo@foo_pkey
 │
 └── • values
       size: 2 columns, 1 row
@@ -196,6 +216,7 @@ vectorized: true
 • relocate table
 │ columns: (key, pretty)
 │ estimated row count: 10 (missing stats)
+│ index: foo@foo_pkey
 │
 └── • values
       columns: (column1, column2)
@@ -210,6 +231,7 @@ distribution: local
 vectorized: true
 ·
 • relocate table
+│ index: foo@a
 │
 └── • values
       size: 2 columns, 1 row
@@ -223,6 +245,7 @@ vectorized: true
 • relocate table
 │ columns: (key, pretty)
 │ estimated row count: 10 (missing stats)
+│ index: foo@a
 │
 └── • values
       columns: (column1, column2)
diff --git a/pkg/sql/opt/exec/execbuilder/testdata/subquery b/pkg/sql/opt/exec/execbuilder/testdata/subquery
index 1e3b205338bb..ec4482a96320 100644
--- a/pkg/sql/opt/exec/execbuilder/testdata/subquery
+++ b/pkg/sql/opt/exec/execbuilder/testdata/subquery
@@ -15,6 +15,8 @@ vectorized: true
 • root
 │
 ├── • split
+│   │ index: abc@abc_pkey
+│   │ expiry: CAST(NULL AS STRING)
 │   │
 │   └── • values
 │         size: 1 column, 1 row
diff --git a/pkg/sql/opt/exec/explain/emit.go b/pkg/sql/opt/exec/explain/emit.go
index e6366c3dc9c2..341eca35f686 100644
--- a/pkg/sql/opt/exec/explain/emit.go
+++ b/pkg/sql/opt/exec/explain/emit.go
@@ -822,6 +822,23 @@ func (e *emitter) emitNodeAttributes(n *Node) error {
 		}
 		e.emitSpans("spans", a.Table, a.Table.Index(cat.PrimaryIndex), params)
 
+	case alterTableSplitOp:
+		a := n.args.(*alterTableSplitArgs)
+		ob.Attrf("index", "%s@%s", a.Index.Table().Name(), a.Index.Name())
+		ob.Expr("expiry", a.Expiration, nil /* columns */)
+
+	case alterTableUnsplitOp:
+		a := n.args.(*alterTableUnsplitArgs)
+		ob.Attrf("index", "%s@%s", a.Index.Table().Name(), a.Index.Name())
+
+	case alterTableUnsplitAllOp:
+		a := n.args.(*alterTableUnsplitAllArgs)
+		ob.Attrf("index", "%s@%s", a.Index.Table().Name(), a.Index.Name())
+
+	case alterTableRelocateOp:
+		a := n.args.(*alterTableRelocateArgs)
+		ob.Attrf("index", "%s@%s", a.Index.Table().Name(), a.Index.Name())
+
 	case recursiveCTEOp:
 		a := n.args.(*recursiveCTEArgs)
 		if e.ob.flags.Verbose && a.Deduplicate {
@@ -852,10 +869,6 @@ func (e *emitter) emitNodeAttributes(n *Node) error {
 		saveTableOp,
 		errorIfRowsOp,
 		opaqueOp,
-		alterTableSplitOp,
-		alterTableUnsplitOp,
-		alterTableUnsplitAllOp,
-		alterTableRelocateOp,
 		controlJobsOp,
 		controlSchedulesOp,
 		cancelQueriesOp,
diff --git a/pkg/sql/opt/exec/explain/testdata/gists b/pkg/sql/opt/exec/explain/testdata/gists
index 042450dba80c..c1c35cac4274 100644
--- a/pkg/sql/opt/exec/explain/testdata/gists
+++ b/pkg/sql/opt/exec/explain/testdata/gists
@@ -862,11 +862,14 @@ hash: 12193483658718214527
 plan-gist: AgICAgYCLGoCBgY=
 explain(shape):
 • split
+│ index: foo@foo_pkey
+│ expiry: CAST(_ AS STRING)
 │
 └── • values
       size: 1 column, 1 row
 explain(gist):
 • split
+│ index: foo@foo_pkey
 │
 └── • values
       size: 1 column, 1 row
@@ -879,11 +882,13 @@ hash: 8861506885543083786
 plan-gist: AgICAgYCLWoCBgQ=
 explain(shape):
 • unsplit
+│ index: foo@foo_pkey
 │
 └── • values
       size: 1 column, 1 row
 explain(gist):
 • unsplit
+│ index: foo@foo_pkey
 │
 └── • values
       size: 1 column, 1 row
@@ -897,8 +902,10 @@ hash: 16533768908468588919
 plan-gist: Ai5qAgYE
 explain(shape):
 • unsplit all
+  index: foo@foo_pkey
 explain(gist):
 • unsplit all
+  index: foo@foo_pkey
 
 # alterTableRelocateOp
 gist-explain-roundtrip
@@ -908,11 +915,13 @@ hash: 8656743312841134920
 plan-gist: AgICBAYEL24CBgQ=
 explain(shape):
 • relocate table
+│ index: abc@abc_pkey
 │
 └── • values
       size: 2 columns, 1 row
 explain(gist):
 • relocate table
+│ index: abc@abc_pkey
 │
 └── • values
       size: 2 columns, 1 row

From 152ab0fda82c948061cdb8b5f07c7b7d459bfb6b Mon Sep 17 00:00:00 2001
From: Marcus Gartner <marcus@cockroachlabs.com>
Date: Thu, 9 Dec 2021 21:23:44 -0500
Subject: [PATCH 2/4] 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                 |  11 ++
 pkg/sql/logictest/testdata/logic_test/cast    |  18 +++
 .../testdata/logic_test/information_schema    |   2 +-
 .../opt/norm/testdata/rules/fold_constants    |   2 +-
 pkg/sql/sem/tree/type_check.go                |   6 +
 pkg/sql/sem/tree/type_check_test.go           |   4 +-
 pkg/sql/types/types.go                        | 107 ++++++++++++++++--
 pkg/sql/types/types_test.go                   |  52 +++++++++
 8 files changed, 189 insertions(+), 13 deletions(-)

diff --git a/pkg/sql/information_schema.go b/pkg/sql/information_schema.go
index 4169eb8a7095..6289e853a60b 100755
--- a/pkg/sql/information_schema.go
+++ b/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 {
diff --git a/pkg/sql/logictest/testdata/logic_test/cast b/pkg/sql/logictest/testdata/logic_test/cast
index 420085700c42..0d56b1679a71 100644
--- a/pkg/sql/logictest/testdata/logic_test/cast
+++ b/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}
 
diff --git a/pkg/sql/logictest/testdata/logic_test/information_schema b/pkg/sql/logictest/testdata/logic_test/information_schema
index 22f47a6f900b..ee4000786a0a 100644
--- a/pkg/sql/logictest/testdata/logic_test/information_schema
+++ b/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
diff --git a/pkg/sql/opt/norm/testdata/rules/fold_constants b/pkg/sql/opt/norm/testdata/rules/fold_constants
index 528bf449945d..58456c50c669 100644
--- a/pkg/sql/opt/norm/testdata/rules/fold_constants
+++ b/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)
diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go
index cc4a14fc415a..55e26a9b2ea7 100644
--- a/pkg/sql/sem/tree/type_check.go
+++ b/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
diff --git a/pkg/sql/sem/tree/type_check_test.go b/pkg/sql/sem/tree/type_check_test.go
index 51d0d3f2cccf..e30c0b7abceb 100644
--- a/pkg/sql/sem/tree/type_check_test.go
+++ b/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)`},
diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go
index 0cc53a7d7841..0aef039407c5 100644
--- a/pkg/sql/types/types.go
+++ b/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 {
diff --git a/pkg/sql/types/types_test.go b/pkg/sql/types/types_test.go
index 37dc54fe70ed..057497e27e62 100644
--- a/pkg/sql/types/types_test.go
+++ b/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())
+		}
+	}
+}

From eb2a6c994b48d47d3fba13a51d83d746007032a9 Mon Sep 17 00:00:00 2001
From: Marcus Gartner <marcus@cockroachlabs.com>
Date: Mon, 13 Dec 2021 15:37:30 -0500
Subject: [PATCH 3/4] opt: eliminate assignment casts with identical source and
 target types

The `EliminateAssignmentCast` rule has been combined with the
`EliminateCast` rule. Now an assignment cast is eliminated if the source
and target type are identical. This now possible thanks to some changes
to type resolution, including:

  1. Placeholder types are resolved with unspecified type modifiers.
     This ensures that assignment casts won't be eliminated if the a
     placeholder value does not conform to the target type's modifiers.

  2. When constant integer `NumVal`s are resolved as a typed-value,
     they are validated to ensure they fit within their type's width.
     There may be more types we need to perform similar validation for,
     such as floats (see #73743).

  3. Columns in values expressions with values that have non-identical
     types but the same type OID will be typed with type modifiers. For
     example, if a values expression has a CHAR(1) value and a CHAR(3)
     value in the same column, the column will be typed as a CHAR
     without an explicit width.

  4. Type modifiers are now correctly omitted from array content types
     when arrays contain constants.

Fixes #73450

Release note (bug fix): A bug has been fixed that caused incorrect
evaluation of placeholder values in EXECUTE statements. The bug
presented when the PREPARE statement cast a placeholder value, for
example `PREPARE s AS SELECT $1::INT2`. If the assigned value for `$1`
exceeded the maximum width value of the cast target type, the result
value of the cast could be incorrect. This bug has been present since
version 19.1 or earlier.
---
 pkg/sql/logictest/testdata/logic_test/cast    | 89 +++++++++++++++++++
 pkg/sql/opt/exec/execbuilder/testdata/insert  | 18 ++--
 pkg/sql/opt/norm/rules/scalar.opt             | 24 ++---
 pkg/sql/opt/norm/scalar_funcs.go              | 28 ------
 .../opt/norm/testdata/rules/fold_constants    |  2 +-
 pkg/sql/opt/norm/testdata/rules/groupby       | 47 +++-------
 pkg/sql/opt/norm/testdata/rules/project       | 17 ++--
 pkg/sql/opt/norm/testdata/rules/scalar        | 37 ++------
 pkg/sql/opt/optbuilder/values.go              |  2 +
 pkg/sql/opt/xform/testdata/external/tpce      | 12 +--
 .../opt/xform/testdata/external/tpce-no-stats | 12 +--
 pkg/sql/opt/xform/testdata/physprops/ordering | 59 ++++++------
 pkg/sql/opt/xform/testdata/rules/groupby      | 34 ++++---
 pkg/sql/sem/tree/cast.go                      |  4 +-
 pkg/sql/sem/tree/constant.go                  |  2 +-
 pkg/sql/sem/tree/testdata/eval/int_arith      | 28 +++++-
 pkg/sql/sem/tree/type_check.go                |  4 +-
 17 files changed, 224 insertions(+), 195 deletions(-)

diff --git a/pkg/sql/logictest/testdata/logic_test/cast b/pkg/sql/logictest/testdata/logic_test/cast
index 0d56b1679a71..c074da9c454a 100644
--- a/pkg/sql/logictest/testdata/logic_test/cast
+++ b/pkg/sql/logictest/testdata/logic_test/cast
@@ -8,6 +8,8 @@ CREATE TABLE assn_cast (
   qc "char",
   b BIT,
   i INT,
+  i2 INT2,
+  f4 FLOAT4,
   t timestamp,
   d DECIMAL(10, 0),
   a DECIMAL(10, 0)[],
@@ -141,6 +143,33 @@ EXECUTE insert_i('1')
 statement error value type string doesn't match type int of column \"i\"
 INSERT INTO assn_cast(i) VALUES ('1'::STRING)
 
+statement error integer out of range for type int2
+INSERT INTO assn_cast(i2) VALUES (999999999)
+
+statement ok
+PREPARE insert_i2 AS INSERT INTO assn_cast(i2) VALUES ($1)
+
+statement error integer out of range for type int2
+EXECUTE insert_i2(99999999)
+
+statement ok
+INSERT INTO assn_cast(f4) VALUES (18754999.99)
+
+statement ok
+PREPARE insert_f4 AS INSERT INTO assn_cast(f4) VALUES ($1)
+
+statement ok
+EXECUTE insert_f4(18754999.99)
+
+# TODO(mgartner): Values are not correctly truncated when cast to FLOAT4, either
+# in an assignment or explicit context. The columns should have a value of
+# 1.8755e+07. See #73743.
+query F
+SELECT f4 FROM assn_cast WHERE f4 IS NOT NULL
+----
+1.875499999e+07
+1.875499999e+07
+
 statement ok
 INSERT INTO assn_cast(t) VALUES ('1970-01-01'::timestamptz)
 
@@ -177,6 +206,15 @@ SELECT d FROM assn_cast WHERE d IS NOT NULL
 123
 68
 
+statement ok
+INSERT INTO assn_cast(a) VALUES (ARRAY[])
+
+statement ok
+INSERT INTO assn_cast(a) VALUES (ARRAY[NULL])
+
+statement ok
+INSERT INTO assn_cast(a) VALUES (ARRAY[1.1])
+
 statement ok
 INSERT INTO assn_cast(a) VALUES (ARRAY[2.88, NULL, 15])
 
@@ -192,13 +230,30 @@ PREPARE insert_a AS INSERT INTO assn_cast(a) VALUES ($1)
 statement ok
 EXECUTE insert_a(ARRAY[7.77, 8.88::DECIMAL(10, 2)])
 
+statement ok
+PREPARE insert_a2 AS INSERT INTO assn_cast(a) VALUES (ARRAY[$1])
+
+statement ok
+EXECUTE insert_a2(20.2)
+
+statement ok
+PREPARE insert_a3 AS INSERT INTO assn_cast(a) VALUES (ARRAY[30.12, $1, 32.1])
+
+statement ok
+EXECUTE insert_a3(30.9)
+
 query T rowsort
 SELECT a FROM assn_cast WHERE a IS NOT NULL
 ----
+{}
+{NULL}
+{1}
 {3,NULL,15}
 {4,NULL,16}
 {6,7}
 {8,9}
+{20}
+{30,31,32}
 
 statement ok
 INSERT INTO assn_cast(s) VALUES (1)
@@ -350,3 +405,37 @@ SELECT '-'::regclass, '-'::regclass::oid,
   '-'::regrole, '-'::regrole::oid
 ----
 -  0  -  0  -  0  -  0  -  0  -  0
+
+# Regression test for #73450. Eliding casts should not cause incorrect results.
+subtest regression_73450
+
+statement ok
+PREPARE s73450_a AS SELECT $1::INT2
+
+statement error integer out of range for type int2
+EXECUTE s73450_a(999999)
+
+statement ok
+PREPARE s73450_b AS SELECT $1::CHAR
+
+query T
+EXECUTE s73450_b('foo')
+----
+f
+
+statement ok
+CREATE TABLE t73450 (c CHAR);
+INSERT INTO t73450 VALUES ('f')
+
+query T
+SELECT * FROM t73450 WHERE c = 'foo'::CHAR
+----
+f
+
+statement ok
+PREPARE s73450_c AS SELECT * FROM t73450 WHERE c = $1::CHAR
+
+query T
+EXECUTE s73450_c('foo')
+----
+f
diff --git a/pkg/sql/opt/exec/execbuilder/testdata/insert b/pkg/sql/opt/exec/execbuilder/testdata/insert
index 1fce8d2b19d2..3db3879cc350 100644
--- a/pkg/sql/opt/exec/execbuilder/testdata/insert
+++ b/pkg/sql/opt/exec/execbuilder/testdata/insert
@@ -318,8 +318,8 @@ vectorized: true
     │ columns: (x, v, rowid_default)
     │ estimated row count: 10 (missing stats)
     │ render rowid_default: unique_rowid()
-    │ render x: crdb_internal.assignment_cast(x, NULL::INT8)
-    │ render v: crdb_internal.assignment_cast(v, NULL::INT8)
+    │ render x: x
+    │ render v: v
     │
     └── • top-k
         │ columns: (x, v)
@@ -355,8 +355,8 @@ vectorized: true
     │ columns: (x, v, rowid_default)
     │ estimated row count: 1 (missing stats)
     │ render rowid_default: unique_rowid()
-    │ render x: crdb_internal.assignment_cast(x, NULL::INT8)
-    │ render v: crdb_internal.assignment_cast(v, NULL::INT8)
+    │ render x: x
+    │ render v: v
     │
     └── • scan
           columns: (x, v)
@@ -471,8 +471,8 @@ vectorized: true
         │ columns: (length, "?column?", rowid_default)
         │ estimated row count: 10 (missing stats)
         │ render rowid_default: unique_rowid()
-        │ render length: crdb_internal.assignment_cast(length, NULL::INT8)
-        │ render ?column?: crdb_internal.assignment_cast("?column?", NULL::INT8)
+        │ render length: length
+        │ render ?column?: "?column?"
         │
         └── • top-k
             │ columns: (length, "?column?", column12)
@@ -600,9 +600,9 @@ vectorized: true
                     │ columns: (a, b, c, rowid_default)
                     │ estimated row count: 1,000 (missing stats)
                     │ render rowid_default: unique_rowid()
-                    │ render a: crdb_internal.assignment_cast(a, NULL::INT8)
-                    │ render b: crdb_internal.assignment_cast(b, NULL::INT8)
-                    │ render c: crdb_internal.assignment_cast(c, NULL::INT8)
+                    │ render a: a
+                    │ render b: b
+                    │ render c: c
                     │
                     └── • scan
                           columns: (a, b, c)
diff --git a/pkg/sql/opt/norm/rules/scalar.opt b/pkg/sql/opt/norm/rules/scalar.opt
index 414293faaf63..ed3b41a1d350 100644
--- a/pkg/sql/opt/norm/rules/scalar.opt
+++ b/pkg/sql/opt/norm/rules/scalar.opt
@@ -58,26 +58,18 @@ $item
 =>
 (SimplifyCoalesce $args)
 
-# EliminateCast discards the cast operator if its input already has a type
-# that's identical to the desired static type.
+# EliminateCast discards a cast or assignment cast operator if its input already
+# has a type that's identical to the desired static type.
 #
 # Note that CastExpr removes unnecessary casts during type-checking; this rule
 # can still be helpful if some other rule creates an unnecessary CastExpr.
-[EliminateCast, Normalize]
-(Cast $input:* $targetTyp:* & (HasColType $input $targetTyp))
-=>
-$input
-
-# EliminateAssignmentCast discards the assignment cast operator if the input
-# type and the target type are identical and the assignment cast is guaranteed
-# to be a no-op. See AssignmentCastIsNoop for an explanation of what makes an
-# assignment cast a no-op.
-[EliminateAssignmentCast, Normalize]
-(AssignmentCast
+#
+# EliminateCast is marked as high-priority so that it matches before
+# FoldAssignmentCast.
+[EliminateCast, Normalize, HighPriority]
+(Cast | AssignmentCast
     $input:*
-    $targetTyp:* &
-        (HasColType $input $targetTyp) &
-        (AssignmentCastIsNoop $targetTyp)
+    $targetTyp:* & (HasColType $input $targetTyp)
 )
 =>
 $input
diff --git a/pkg/sql/opt/norm/scalar_funcs.go b/pkg/sql/opt/norm/scalar_funcs.go
index 51d30a9aa12e..28fe50019d5a 100644
--- a/pkg/sql/opt/norm/scalar_funcs.go
+++ b/pkg/sql/opt/norm/scalar_funcs.go
@@ -368,31 +368,3 @@ func (c *CustomFuncs) SplitTupleEq(lhs, rhs *memo.TupleExpr) memo.FiltersExpr {
 	}
 	return res
 }
-
-// AssignmentCastIsNoop returns true if an assignment cast from t to t is
-// guaranteed to be a no-op: it never alters the input value and never errors.
-// These conditions must hold in the unintuitive, but crucial case where a value
-// does not fit within the width of t.
-//
-// The width of a value may be wider than its type's width when executing
-// prepared statements. For example, no width validation is performed when
-// assigning a string to a placeholder that has been typed as a CHAR(1). An
-// assignment cast from a value with more than one character, even if typed as a
-// CHAR(1), to a CHAR(1) will result in an error, so it is not guaranteed to be
-// a no-op.
-//
-// Currently, AssignmentCastIsNoop returns true only for the UUID type. UUIDs
-// have no width modifier, and values assigned to UUID placeholders are
-// validated, so an assignment cast from UUID to UUID is guaranteed to be a
-// no-op.
-//
-// TODO(mgartner): There are other types for which assignment casts are
-// guaranteed no-ops. We should expand this list.
-func (c *CustomFuncs) AssignmentCastIsNoop(t *types.T) bool {
-	switch t.Family() {
-	case types.UuidFamily:
-		return true
-	default:
-		return false
-	}
-}
diff --git a/pkg/sql/opt/norm/testdata/rules/fold_constants b/pkg/sql/opt/norm/testdata/rules/fold_constants
index 58456c50c669..2a679a63619a 100644
--- a/pkg/sql/opt/norm/testdata/rules/fold_constants
+++ b/pkg/sql/opt/norm/testdata/rules/fold_constants
@@ -905,7 +905,7 @@ insert assn_cast
 
 # Do not fold non-constants even if they have an identical type as the target
 # type.
-norm expect-not=FoldAssignmentCast
+norm expect-not=FoldAssignmentCast disable=EliminateCast
 INSERT INTO assn_cast(c) VALUES ($1::CHAR)
 ----
 insert assn_cast
diff --git a/pkg/sql/opt/norm/testdata/rules/groupby b/pkg/sql/opt/norm/testdata/rules/groupby
index d41cb8825601..1685fafe2214 100644
--- a/pkg/sql/opt/norm/testdata/rules/groupby
+++ b/pkg/sql/opt/norm/testdata/rules/groupby
@@ -953,17 +953,14 @@ insert xy
  └── limit
       ├── columns: y:9!null y_default:10
       ├── cardinality: [0 - 1]
-      ├── immutable
       ├── key: ()
       ├── fd: ()-->(9,10)
       ├── anti-join (hash)
       │    ├── columns: y:9!null y_default:10
-      │    ├── immutable
       │    ├── fd: ()-->(9,10)
       │    ├── limit hint: 1.00
       │    ├── project
       │    │    ├── columns: y_default:10 y:9!null
-      │    │    ├── immutable
       │    │    ├── fd: ()-->(9,10)
       │    │    ├── select
       │    │    │    ├── columns: xy.y:6!null
@@ -974,8 +971,7 @@ insert xy
       │    │    │         └── xy.y:6 = 0 [outer=(6), constraints=(/6: [/0 - /0]; tight), fd=()-->(6)]
       │    │    └── projections
       │    │         ├── CAST(NULL AS INT8) [as=y_default:10]
-      │    │         └── assignment-cast: INT8 [as=y:9, outer=(6), immutable]
-      │    │              └── xy.y:6
+      │    │         └── xy.y:6 [as=y:9, outer=(6)]
       │    ├── scan xy
       │    │    ├── columns: x:11!null
       │    │    └── key: (11)
@@ -1056,16 +1052,13 @@ insert xy
  └── upsert-distinct-on
       ├── columns: y:9 y_default:10
       ├── grouping columns: y:9
-      ├── immutable
       ├── lax-key: (9)
       ├── fd: ()-->(9,10)
       ├── anti-join (hash)
       │    ├── columns: y:9 y_default:10
-      │    ├── immutable
       │    ├── fd: ()-->(9,10)
       │    ├── project
       │    │    ├── columns: y_default:10 y:9
-      │    │    ├── immutable
       │    │    ├── fd: ()-->(9,10)
       │    │    ├── select
       │    │    │    ├── columns: xy.y:6
@@ -1076,8 +1069,7 @@ insert xy
       │    │    │         └── xy.y:6 IS NULL [outer=(6), constraints=(/6: [/NULL - /NULL]; tight), fd=()-->(6)]
       │    │    └── projections
       │    │         ├── CAST(NULL AS INT8) [as=y_default:10]
-      │    │         └── assignment-cast: INT8 [as=y:9, outer=(6), immutable]
-      │    │              └── xy.y:6
+      │    │         └── xy.y:6 [as=y:9, outer=(6)]
       │    ├── scan xy
       │    │    ├── columns: x:11!null
       │    │    └── key: (11)
@@ -1687,17 +1679,14 @@ insert a
  └── limit
       ├── columns: "?column?":17!null i:18!null "?column?":19!null f_default:20 j_default:21
       ├── cardinality: [0 - 1]
-      ├── immutable
       ├── key: ()
       ├── fd: ()-->(17-21)
       ├── anti-join (hash)
       │    ├── columns: "?column?":17!null i:18!null "?column?":19!null f_default:20 j_default:21
-      │    ├── immutable
       │    ├── fd: ()-->(17-21)
       │    ├── limit hint: 1.00
       │    ├── project
       │    │    ├── columns: f_default:20 j_default:21 "?column?":17!null i:18!null "?column?":19!null
-      │    │    ├── immutable
       │    │    ├── fd: ()-->(17-21)
       │    │    ├── select
       │    │    │    ├── columns: a.i:9!null
@@ -1710,8 +1699,7 @@ insert a
       │    │         ├── CAST(NULL AS FLOAT8) [as=f_default:20]
       │    │         ├── CAST(NULL AS JSONB) [as=j_default:21]
       │    │         ├── 1 [as="?column?":17]
-      │    │         ├── assignment-cast: INT8 [as=i:18, outer=(9), immutable]
-      │    │         │    └── a.i:9
+      │    │         ├── a.i:9 [as=i:18, outer=(9)]
       │    │         └── 'foo' [as="?column?":19]
       │    ├── select
       │    │    ├── columns: a.i:23!null s:25!null
@@ -2465,18 +2453,8 @@ insert a
       │    │    │    │    │    ├── columns: column1:12 column2:13!null column3:14!null column4:15!null
       │    │    │    │    │    ├── cardinality: [2 - 2]
       │    │    │    │    │    ├── volatile
-      │    │    │    │    │    ├── tuple
-      │    │    │    │    │    │    ├── assignment-cast: INT8
-      │    │    │    │    │    │    │    └── unique_rowid()
-      │    │    │    │    │    │    ├── 'foo'
-      │    │    │    │    │    │    ├── 1
-      │    │    │    │    │    │    └── 1.0
-      │    │    │    │    │    └── tuple
-      │    │    │    │    │         ├── assignment-cast: INT8
-      │    │    │    │    │         │    └── unique_rowid()
-      │    │    │    │    │         ├── 'bar'
-      │    │    │    │    │         ├── 2
-      │    │    │    │    │         └── 2.0
+      │    │    │    │    │    ├── (unique_rowid(), 'foo', 1, 1.0)
+      │    │    │    │    │    └── (unique_rowid(), 'bar', 2, 2.0)
       │    │    │    │    └── projections
       │    │    │    │         └── CAST(NULL AS JSONB) [as=j_default:16]
       │    │    │    ├── scan a
@@ -2526,27 +2504,22 @@ insert a
  └── upsert-distinct-on
       ├── columns: i:16!null "?column?":17!null i:18!null f_default:19 j_default:20
       ├── grouping columns: i:18!null
-      ├── immutable
       ├── key: (18)
-      ├── fd: ()-->(17,19,20), (18)-->(16,17,19,20)
+      ├── fd: ()-->(17,19,20), (16)==(18), (18)==(16), (18)-->(16,17,19,20)
       ├── anti-join (hash)
       │    ├── columns: i:16!null "?column?":17!null i:18!null f_default:19 j_default:20
-      │    ├── immutable
-      │    ├── fd: ()-->(17,19,20)
+      │    ├── fd: ()-->(17,19,20), (16)==(18), (18)==(16)
       │    ├── project
       │    │    ├── columns: f_default:19 j_default:20 i:16!null "?column?":17!null i:18!null
-      │    │    ├── immutable
-      │    │    ├── fd: ()-->(17,19,20)
+      │    │    ├── fd: ()-->(17,19,20), (16)==(18), (18)==(16)
       │    │    ├── scan a
       │    │    │    └── columns: a.i:9!null
       │    │    └── projections
       │    │         ├── CAST(NULL AS FLOAT8) [as=f_default:19]
       │    │         ├── CAST(NULL AS JSONB) [as=j_default:20]
-      │    │         ├── assignment-cast: INT8 [as=i:16, outer=(9), immutable]
-      │    │         │    └── a.i:9
+      │    │         ├── a.i:9 [as=i:16, outer=(9)]
       │    │         ├── 'foo' [as="?column?":17]
-      │    │         └── assignment-cast: INT8 [as=i:18, outer=(9), immutable]
-      │    │              └── a.i:9
+      │    │         └── a.i:9 [as=i:18, outer=(9)]
       │    ├── select
       │    │    ├── columns: a.i:22!null s:24!null
       │    │    ├── key: (22)
diff --git a/pkg/sql/opt/norm/testdata/rules/project b/pkg/sql/opt/norm/testdata/rules/project
index 7dcefe0eb188..9fab2423be16 100644
--- a/pkg/sql/opt/norm/testdata/rules/project
+++ b/pkg/sql/opt/norm/testdata/rules/project
@@ -1103,10 +1103,8 @@ insert assn_cast
       │         │    └── $1
       │         ├── assignment-cast: "char"
       │         │    └── $2
-      │         ├── assignment-cast: INT8
-      │         │    └── $3
-      │         └── assignment-cast: STRING
-      │              └── $4
+      │         ├── $3
+      │         └── $4
       └── projections
            └── unique_rowid() [as=rowid_default:16, volatile]
 
@@ -1158,12 +1156,12 @@ insert assn_cast
  ├── cardinality: [0 - 0]
  ├── volatile, mutations
  └── project
-      ├── columns: c_default:12 qc_default:13 rowid_default:14 x:10!null text:11!null
+      ├── columns: c_default:12 qc_default:13 rowid_default:14 text:11!null x:10!null
       ├── cardinality: [2 - 2]
       ├── volatile
-      ├── fd: ()-->(12,13)
+      ├── fd: ()-->(12,13), (10)-->(11)
       ├── values
-      │    ├── columns: column1:8!null
+      │    ├── columns: x:10!null
       │    ├── cardinality: [2 - 2]
       │    ├── (1,)
       │    └── (2,)
@@ -1171,10 +1169,7 @@ insert assn_cast
            ├── CAST(NULL AS CHAR) [as=c_default:12]
            ├── CAST(NULL AS "char") [as=qc_default:13]
            ├── unique_rowid() [as=rowid_default:14, volatile]
-           ├── assignment-cast: INT8 [as=x:10, outer=(8), immutable]
-           │    └── column1:8
-           └── assignment-cast: STRING [as=text:11, outer=(8), immutable]
-                └── (column1:8 + 1)::STRING
+           └── (x:10 + 1)::STRING [as=text:11, outer=(10), immutable]
 
 # Do not match if there are no assignment cast projections.
 norm expect-not=PushAssignmentCastsIntoValues
diff --git a/pkg/sql/opt/norm/testdata/rules/scalar b/pkg/sql/opt/norm/testdata/rules/scalar
index eba5d08c25f8..bfe15ee0a46d 100644
--- a/pkg/sql/opt/norm/testdata/rules/scalar
+++ b/pkg/sql/opt/norm/testdata/rules/scalar
@@ -225,11 +225,8 @@ project
       ├── ARRAY[a.i:2, 2]::OIDVECTOR [as=array:14, outer=(2), stable]
       └── ARRAY[a.i:2, 2]::INT2VECTOR [as=array:15, outer=(2), immutable]
 
-# --------------------------------------------------
-# EliminateAssignmentCast
-# --------------------------------------------------
-
-norm expect=EliminateAssignmentCast
+# Eliminate assignment casts.
+norm expect=EliminateCast
 INSERT INTO u VALUES ('b1621d9e-df89-4e8a-89f5-82262bca907b')
 ----
 insert u
@@ -245,7 +242,8 @@ insert u
       ├── fd: ()-->(5)
       └── ('b1621d9e-df89-4e8a-89f5-82262bca907b',)
 
-norm expect=EliminateAssignmentCast
+# Eliminate assignment casts.
+norm expect=EliminateCast
 INSERT INTO u VALUES (gen_random_uuid())
 ----
 insert u
@@ -263,7 +261,7 @@ insert u
       └── (gen_random_uuid(),)
 
 # Do not eliminate assignment casts when the input type does not match the target type.
-norm expect-not=EliminateAssignmentCast disable=FoldAssignmentCast
+norm disable=FoldAssignmentCast
 INSERT INTO a (k, s) VALUES (1, 'b1621d9e-df89-4e8a-89f5-82262bca907b'::UUID)
 ----
 insert a
@@ -277,40 +275,19 @@ insert a
  ├── cardinality: [0 - 0]
  ├── volatile, mutations
  └── values
-      ├── columns: column1:10 column2:11 i_default:12 f_default:13 arr_default:14
+      ├── columns: column1:10!null column2:11 i_default:12 f_default:13 arr_default:14
       ├── cardinality: [1 - 1]
       ├── immutable
       ├── key: ()
       ├── fd: ()-->(10-14)
       └── tuple
-           ├── assignment-cast: INT8
-           │    └── 1
+           ├── 1
            ├── assignment-cast: STRING
            │    └── 'b1621d9e-df89-4e8a-89f5-82262bca907b'
            ├── CAST(NULL AS INT8)
            ├── CAST(NULL AS FLOAT8)
            └── CAST(NULL AS INT8[])
 
-# Do not eliminate assignment casts for the CHAR type.
-norm expect-not=EliminateAssignmentCast disable=FoldAssignmentCast
-INSERT INTO c VALUES ('f'::CHAR)
-----
-insert c
- ├── columns: <none>
- ├── insert-mapping:
- │    └── column1:5 => c:1
- ├── cardinality: [0 - 0]
- ├── volatile, mutations
- └── values
-      ├── columns: column1:5
-      ├── cardinality: [1 - 1]
-      ├── immutable
-      ├── key: ()
-      ├── fd: ()-->(5)
-      └── tuple
-           └── assignment-cast: CHAR
-                └── 'f'
-
 # --------------------------------------------------
 # NormalizeInConst
 # --------------------------------------------------
diff --git a/pkg/sql/opt/optbuilder/values.go b/pkg/sql/opt/optbuilder/values.go
index 1779d6697dfb..f4d2597aa096 100644
--- a/pkg/sql/opt/optbuilder/values.go
+++ b/pkg/sql/opt/optbuilder/values.go
@@ -86,6 +86,8 @@ func (b *Builder) buildValuesClause(
 				} else if !typ.Equivalent(colTypes[colIdx]) {
 					panic(pgerror.Newf(pgcode.DatatypeMismatch,
 						"VALUES types %s and %s cannot be matched", typ, colTypes[colIdx]))
+				} else if !typ.Identical(colTypes[colIdx]) {
+					colTypes[colIdx] = colTypes[colIdx].WithoutTypeModifiers()
 				}
 			}
 			elems[elemPos] = b.buildScalar(texpr, inScope, nil, nil, nil)
diff --git a/pkg/sql/opt/xform/testdata/external/tpce b/pkg/sql/opt/xform/testdata/external/tpce
index b617e724b0ab..d2b99c7655d7 100644
--- a/pkg/sql/opt/xform/testdata/external/tpce
+++ b/pkg/sql/opt/xform/testdata/external/tpce
@@ -677,10 +677,11 @@ with &2 (update_last_trade)
                 │    │    │    └── "?column?":63 => trade_history.th_st_id:53
                 │    │    ├── input binding: &5
                 │    │    ├── volatile, mutations
+                │    │    ├── key: (51)
                 │    │    ├── fd: ()-->(53)
                 │    │    ├── project
                 │    │    │    ├── columns: tr_t_id:62!null "?column?":63!null timestamp:64!null
-                │    │    │    ├── immutable
+                │    │    │    ├── key: (62)
                 │    │    │    ├── fd: ()-->(63,64)
                 │    │    │    ├── with-scan &3 (request_list)
                 │    │    │    │    ├── columns: tr_t_id:56!null
@@ -688,8 +689,7 @@ with &2 (update_last_trade)
                 │    │    │    │    │    └──  trade_request.tr_t_id:20 => tr_t_id:56
                 │    │    │    │    └── key: (56)
                 │    │    │    └── projections
-                │    │    │         ├── assignment-cast: INT8 [as=tr_t_id:62, outer=(56), immutable]
-                │    │    │         │    └── tr_t_id:56
+                │    │    │         ├── tr_t_id:56 [as=tr_t_id:62, outer=(56)]
                 │    │    │         ├── 'SBMT' [as="?column?":63]
                 │    │    │         └── '2020-06-15 22:27:42.148484' [as=timestamp:64]
                 │    │    └── f-k-checks
@@ -698,10 +698,12 @@ with &2 (update_last_trade)
                 │    │         │         ├── columns: th_t_id:65!null
                 │    │         │         ├── key columns: [65] = [66]
                 │    │         │         ├── lookup columns are key
+                │    │         │         ├── key: (65)
                 │    │         │         ├── with-scan &5
                 │    │         │         │    ├── columns: th_t_id:65!null
-                │    │         │         │    └── mapping:
-                │    │         │         │         └──  tr_t_id:62 => th_t_id:65
+                │    │         │         │    ├── mapping:
+                │    │         │         │    │    └──  tr_t_id:62 => th_t_id:65
+                │    │         │         │    └── key: (65)
                 │    │         │         └── filters (true)
                 │    │         └── f-k-checks-item: trade_history(th_st_id) -> status_type(st_id)
                 │    │              └── anti-join (lookup status_type)
diff --git a/pkg/sql/opt/xform/testdata/external/tpce-no-stats b/pkg/sql/opt/xform/testdata/external/tpce-no-stats
index 1135486b38db..fa50fcc0c3b4 100644
--- a/pkg/sql/opt/xform/testdata/external/tpce-no-stats
+++ b/pkg/sql/opt/xform/testdata/external/tpce-no-stats
@@ -695,10 +695,11 @@ with &2 (update_last_trade)
                 │    │    │    └── "?column?":63 => trade_history.th_st_id:53
                 │    │    ├── input binding: &5
                 │    │    ├── volatile, mutations
+                │    │    ├── key: (51)
                 │    │    ├── fd: ()-->(53)
                 │    │    ├── project
                 │    │    │    ├── columns: tr_t_id:62!null "?column?":63!null timestamp:64!null
-                │    │    │    ├── immutable
+                │    │    │    ├── key: (62)
                 │    │    │    ├── fd: ()-->(63,64)
                 │    │    │    ├── with-scan &3 (request_list)
                 │    │    │    │    ├── columns: tr_t_id:56!null
@@ -706,8 +707,7 @@ with &2 (update_last_trade)
                 │    │    │    │    │    └──  trade_request.tr_t_id:20 => tr_t_id:56
                 │    │    │    │    └── key: (56)
                 │    │    │    └── projections
-                │    │    │         ├── assignment-cast: INT8 [as=tr_t_id:62, outer=(56), immutable]
-                │    │    │         │    └── tr_t_id:56
+                │    │    │         ├── tr_t_id:56 [as=tr_t_id:62, outer=(56)]
                 │    │    │         ├── 'SBMT' [as="?column?":63]
                 │    │    │         └── '2020-06-15 22:27:42.148484' [as=timestamp:64]
                 │    │    └── f-k-checks
@@ -716,10 +716,12 @@ with &2 (update_last_trade)
                 │    │         │         ├── columns: th_t_id:65!null
                 │    │         │         ├── key columns: [65] = [66]
                 │    │         │         ├── lookup columns are key
+                │    │         │         ├── key: (65)
                 │    │         │         ├── with-scan &5
                 │    │         │         │    ├── columns: th_t_id:65!null
-                │    │         │         │    └── mapping:
-                │    │         │         │         └──  tr_t_id:62 => th_t_id:65
+                │    │         │         │    ├── mapping:
+                │    │         │         │    │    └──  tr_t_id:62 => th_t_id:65
+                │    │         │         │    └── key: (65)
                 │    │         │         └── filters (true)
                 │    │         └── f-k-checks-item: trade_history(th_st_id) -> status_type(st_id)
                 │    │              └── anti-join (lookup status_type)
diff --git a/pkg/sql/opt/xform/testdata/physprops/ordering b/pkg/sql/opt/xform/testdata/physprops/ordering
index 3bd75ecf1e64..c74fb53d3f4e 100644
--- a/pkg/sql/opt/xform/testdata/physprops/ordering
+++ b/pkg/sql/opt/xform/testdata/physprops/ordering
@@ -1759,11 +1759,13 @@ sort
  ├── columns: a:14!null b:15!null c:16!null
  ├── cardinality: [0 - 2]
  ├── volatile, mutations
+ ├── key: (14-16)
  ├── ordering: +15
  └── with &1
       ├── columns: a:14!null b:15!null c:16!null
       ├── cardinality: [0 - 2]
       ├── volatile, mutations
+      ├── key: (14-16)
       ├── insert abc
       │    ├── columns: abc.a:1!null abc.b:2!null abc.c:3!null
       │    ├── insert-mapping:
@@ -1772,10 +1774,11 @@ sort
       │    │    └── z:13 => abc.c:3
       │    ├── cardinality: [0 - 2]
       │    ├── volatile, mutations
+      │    ├── key: (1-3)
       │    └── project
       │         ├── columns: x:11!null y:12!null z:13!null
       │         ├── cardinality: [0 - 2]
-      │         ├── immutable
+      │         ├── key: (11-13)
       │         ├── top-k
       │         │    ├── columns: xyz.x:6!null xyz.y:7!null xyz.z:8!null
       │         │    ├── internal-ordering: +7,+8
@@ -1786,19 +1789,17 @@ sort
       │         │         ├── columns: xyz.x:6!null xyz.y:7!null xyz.z:8!null
       │         │         └── key: (6-8)
       │         └── projections
-      │              ├── assignment-cast: INT8 [as=x:11, outer=(6), immutable]
-      │              │    └── xyz.x:6
-      │              ├── assignment-cast: INT8 [as=y:12, outer=(7), immutable]
-      │              │    └── xyz.y:7
-      │              └── assignment-cast: INT8 [as=z:13, outer=(8), immutable]
-      │                   └── xyz.z:8
+      │              ├── xyz.x:6 [as=x:11, outer=(6)]
+      │              ├── xyz.y:7 [as=y:12, outer=(7)]
+      │              └── xyz.z:8 [as=z:13, outer=(8)]
       └── with-scan &1
            ├── columns: a:14!null b:15!null c:16!null
            ├── mapping:
            │    ├──  abc.a:1 => a:14
            │    ├──  abc.b:2 => b:15
            │    └──  abc.c:3 => c:16
-           └── cardinality: [0 - 2]
+           ├── cardinality: [0 - 2]
+           └── key: (14-16)
 
 # Verify that provided orderings are derived correctly.
 opt
@@ -1826,17 +1827,13 @@ sort
       │    └── project
       │         ├── columns: b:13 c:14 d:15
       │         ├── cardinality: [0 - 2]
-      │         ├── immutable
       │         ├── scan abcd@cd
       │         │    ├── columns: abcd.b:7 abcd.c:8 abcd.d:9
       │         │    └── limit: 2
       │         └── projections
-      │              ├── assignment-cast: INT8 [as=b:13, outer=(7), immutable]
-      │              │    └── abcd.b:7
-      │              ├── assignment-cast: INT8 [as=c:14, outer=(8), immutable]
-      │              │    └── abcd.c:8
-      │              └── assignment-cast: INT8 [as=d:15, outer=(9), immutable]
-      │                   └── abcd.d:9
+      │              ├── abcd.b:7 [as=b:13, outer=(7)]
+      │              ├── abcd.c:8 [as=c:14, outer=(8)]
+      │              └── abcd.d:9 [as=d:15, outer=(9)]
       └── with-scan &1
            ├── columns: x:16!null y:17!null z:18!null
            ├── mapping:
@@ -1875,17 +1872,13 @@ sort
       │    └── project
       │         ├── columns: b:13 c:14 d:15
       │         ├── cardinality: [0 - 2]
-      │         ├── immutable
       │         ├── scan abcd@cd
       │         │    ├── columns: abcd.b:7 abcd.c:8 abcd.d:9
       │         │    └── limit: 2
       │         └── projections
-      │              ├── assignment-cast: INT8 [as=b:13, outer=(7), immutable]
-      │              │    └── abcd.b:7
-      │              ├── assignment-cast: INT8 [as=c:14, outer=(8), immutable]
-      │              │    └── abcd.c:8
-      │              └── assignment-cast: INT8 [as=d:15, outer=(9), immutable]
-      │                   └── abcd.d:9
+      │              ├── abcd.b:7 [as=b:13, outer=(7)]
+      │              ├── abcd.c:8 [as=c:14, outer=(8)]
+      │              └── abcd.d:9 [as=d:15, outer=(9)]
       └── select
            ├── columns: x:16!null y:17!null z:18!null
            ├── cardinality: [0 - 2]
@@ -1907,6 +1900,7 @@ SELECT * FROM [INSERT INTO abc SELECT * FROM xyz ORDER BY y, z RETURNING *]
 with &1
  ├── columns: a:14!null b:15!null c:16!null
  ├── volatile, mutations
+ ├── key: (14-16)
  ├── insert abc
  │    ├── columns: abc.a:1!null abc.b:2!null abc.c:3!null
  │    ├── insert-mapping:
@@ -1914,25 +1908,24 @@ with &1
  │    │    ├── y:12 => abc.b:2
  │    │    └── z:13 => abc.c:3
  │    ├── volatile, mutations
+ │    ├── key: (1-3)
  │    └── project
  │         ├── columns: x:11!null y:12!null z:13!null
- │         ├── immutable
+ │         ├── key: (11-13)
  │         ├── scan xyz
  │         │    ├── columns: xyz.x:6!null xyz.y:7!null xyz.z:8!null
  │         │    └── key: (6-8)
  │         └── projections
- │              ├── assignment-cast: INT8 [as=x:11, outer=(6), immutable]
- │              │    └── xyz.x:6
- │              ├── assignment-cast: INT8 [as=y:12, outer=(7), immutable]
- │              │    └── xyz.y:7
- │              └── assignment-cast: INT8 [as=z:13, outer=(8), immutable]
- │                   └── xyz.z:8
+ │              ├── xyz.x:6 [as=x:11, outer=(6)]
+ │              ├── xyz.y:7 [as=y:12, outer=(7)]
+ │              └── xyz.z:8 [as=z:13, outer=(8)]
  └── with-scan &1
       ├── columns: a:14!null b:15!null c:16!null
-      └── mapping:
-           ├──  abc.a:1 => a:14
-           ├──  abc.b:2 => b:15
-           └──  abc.c:3 => c:16
+      ├── mapping:
+      │    ├──  abc.a:1 => a:14
+      │    ├──  abc.b:2 => b:15
+      │    └──  abc.c:3 => c:16
+      └── key: (14-16)
 
 # --------------------------------------------------
 # Update operator.
diff --git a/pkg/sql/opt/xform/testdata/rules/groupby b/pkg/sql/opt/xform/testdata/rules/groupby
index a3b59a4c4088..c35a1206005b 100644
--- a/pkg/sql/opt/xform/testdata/rules/groupby
+++ b/pkg/sql/opt/xform/testdata/rules/groupby
@@ -2246,27 +2246,36 @@ memo (optimized, ~32KB, required=[presentation: w:12] [ordering: +13,+1])
 memo
 INSERT INTO xyz SELECT v, w, 1.0 FROM kuvw ON CONFLICT (x) DO NOTHING
 ----
-memo (optimized, ~24KB, required=[])
+memo (optimized, ~26KB, required=[])
  ├── G1: (insert G2 G3 G4 xyz)
  │    └── []
  │         ├── best: (insert G2 G3 G4 xyz)
- │         └── cost: 2219.17
- ├── G2: (upsert-distinct-on G5 G6 cols=(13))
+ │         └── cost: 2209.02
+ ├── G2: (upsert-distinct-on G5 G6 cols=(13)) (upsert-distinct-on G5 G6 cols=(13),ordering=+13 opt(15))
  │    └── []
  │         ├── best: (upsert-distinct-on G5 G6 cols=(13))
- │         └── cost: 2219.16
+ │         └── cost: 2209.01
  ├── G3: (unique-checks)
  ├── G4: (f-k-checks)
- ├── G5: (anti-join G7 G8 G9) (lookup-join G7 G10 xyz,keyCols=[13],outCols=(13-16)) (lookup-join G7 G10 xyz@xy,keyCols=[13],outCols=(13-16))
+ ├── G5: (anti-join G7 G8 G9) (merge-join G7 G8 G10 anti-join,+13,+16) (lookup-join G7 G10 xyz,keyCols=[13],outCols=(13-16)) (lookup-join G7 G10 xyz@xy,keyCols=[13],outCols=(13-16))
+ │    ├── [ordering: +13 opt(15)]
+ │    │    ├── best: (merge-join G7="[ordering: +13 opt(15)]" G8="[ordering: +16]" G10 anti-join,+13,+16)
+ │    │    └── cost: 2208.98
  │    └── []
- │         ├── best: (anti-join G7 G8 G9)
- │         └── cost: 2219.13
+ │         ├── best: (merge-join G7="[ordering: +13 opt(15)]" G8="[ordering: +16]" G10 anti-join,+13,+16)
+ │         └── cost: 2208.98
  ├── G6: (aggregations G11 G12)
  ├── G7: (project G13 G14)
+ │    ├── [ordering: +13 opt(15)]
+ │    │    ├── best: (project G13="[ordering: +8]" G14)
+ │    │    └── cost: 1124.64
  │    └── []
  │         ├── best: (project G13 G14)
  │         └── cost: 1124.64
  ├── G8: (scan xyz,cols=(16)) (scan xyz@xy,cols=(16)) (scan xyz@zyx,cols=(16)) (scan xyz@yy,cols=(16))
+ │    ├── [ordering: +16]
+ │    │    ├── best: (scan xyz@xy,cols=(16))
+ │    │    └── cost: 1054.32
  │    └── []
  │         ├── best: (scan xyz@xy,cols=(16))
  │         └── cost: 1054.32
@@ -2275,6 +2284,9 @@ memo (optimized, ~24KB, required=[])
  ├── G11: (first-agg G16)
  ├── G12: (first-agg G17)
  ├── G13: (scan kuvw,cols=(8,9)) (scan kuvw@uvw,cols=(8,9)) (scan kuvw@wvu,cols=(8,9)) (scan kuvw@vw,cols=(8,9)) (scan kuvw@w,cols=(8,9))
+ │    ├── [ordering: +8]
+ │    │    ├── best: (scan kuvw@vw,cols=(8,9))
+ │    │    └── cost: 1084.62
  │    └── []
  │         ├── best: (scan kuvw,cols=(8,9))
  │         └── cost: 1084.62
@@ -2282,13 +2294,11 @@ memo (optimized, ~24KB, required=[])
  ├── G15: (eq G21 G22)
  ├── G16: (variable w)
  ├── G17: (variable "?column?")
- ├── G18: (assignment-cast G23)
- ├── G19: (assignment-cast G24)
+ ├── G18: (variable kuvw.v)
+ ├── G19: (variable kuvw.w)
  ├── G20: (const 1.0)
  ├── G21: (variable v)
- ├── G22: (variable x)
- ├── G23: (variable kuvw.v)
- └── G24: (variable kuvw.w)
+ └── G22: (variable x)
 
 # Ensure that streaming ensure-upsert-distinct-on will be used.
 memo
diff --git a/pkg/sql/sem/tree/cast.go b/pkg/sql/sem/tree/cast.go
index 9e70247c4b76..83c26a90c3dc 100644
--- a/pkg/sql/sem/tree/cast.go
+++ b/pkg/sql/sem/tree/cast.go
@@ -788,8 +788,8 @@ func ForEachCast(fn func(src, tgt oid.Oid)) {
 // ValidCast returns true if a valid cast exists from src to tgt in the given
 // context.
 func ValidCast(src, tgt *types.T, ctx CastContext) bool {
-	// If src and tgt are identical, a cast is valid in any context.
-	if src.Identical(tgt) {
+	// If src and tgt are the same type, a cast is valid in any context.
+	if src.Oid() == tgt.Oid() {
 		return true
 	}
 
diff --git a/pkg/sql/sem/tree/constant.go b/pkg/sql/sem/tree/constant.go
index 084aecc4405b..ab12cf2dca45 100644
--- a/pkg/sql/sem/tree/constant.go
+++ b/pkg/sql/sem/tree/constant.go
@@ -305,7 +305,7 @@ func (expr *NumVal) ResolveAsType(
 				return nil, err
 			}
 		}
-		return &expr.resInt, nil
+		return AdjustValueToType(typ, &expr.resInt)
 	case types.FloatFamily:
 		if strings.EqualFold(expr.origString, "NaN") {
 			// We need to check NaN separately since expr.value is unknownVal for NaN.
diff --git a/pkg/sql/sem/tree/testdata/eval/int_arith b/pkg/sql/sem/tree/testdata/eval/int_arith
index 49f5e43f19d8..d4abf790ee31 100644
--- a/pkg/sql/sem/tree/testdata/eval/int_arith
+++ b/pkg/sql/sem/tree/testdata/eval/int_arith
@@ -123,15 +123,25 @@ eval
 ----
 integer out of range
 
+eval
+-32767:::int2 * -1:::int2
+----
+32767
+
 eval
 -32768:::int2 * -1:::int2
 ----
-32768
+integer out of range for type int2
+
+eval
+-2147483647:::int4 * -1:::int4
+----
+2147483647
 
 eval
 -2147483648:::int4 * -1:::int4
 ----
-2147483648
+integer out of range for type int4
 
 eval
 (-9223372036854775807:::int8 - 1) * -1:::int8
@@ -172,15 +182,25 @@ eval
 # Integer division. #
 #####################
 
+eval
+-32767:::int2 / -1:::int2
+----
+32767
+
 eval
 -32768:::int2 / -1:::int2
 ----
-32768
+integer out of range for type int2
+
+eval
+-2147483647:::int4 / -1:::int4
+----
+2147483647
 
 eval
 -2147483648:::int4 / -1:::int4
 ----
-2147483648
+integer out of range for type int4
 
 eval
 (-9223372036854775807:::int8 - 1) / -1:::int8
diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go
index 55e26a9b2ea7..1ccb149af243 100644
--- a/pkg/sql/sem/tree/type_check.go
+++ b/pkg/sql/sem/tree/type_check.go
@@ -2377,7 +2377,9 @@ func typeCheckSameTypedConsts(
 			}
 		}
 		if all {
-			return setTypeForConsts(typ)
+			// Constants do not have types with modifiers so clear the modifiers
+			// of typ before setting it.
+			return setTypeForConsts(typ.WithoutTypeModifiers())
 		}
 	}
 

From 4b3847231f2cac1b42b89ee46e6ce3076cb3209c Mon Sep 17 00:00:00 2001
From: Ricky Stewart <ricky@cockroachlabs.com>
Date: Tue, 14 Dec 2021 14:21:17 -0600
Subject: [PATCH 4/4] bazel: upgrade `rules_foreign_cc` to 0.7

Also add `-fcommon` to compile flags for `krb5`.

Closes #71306.

Release note: None
---
 .bazelrc           |  11 +----
 WORKSPACE          |   7 +--
 c-deps/BUILD.bazel | 103 +++++++++++++++++++--------------------------
 3 files changed, 49 insertions(+), 72 deletions(-)

diff --git a/.bazelrc b/.bazelrc
index 60d456ec92e4..e8828ee87518 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -1,6 +1,6 @@
 try-import %workspace%/.bazelrc.user
 
-build --symlink_prefix=_bazel/ --ui_event_filters=-DEBUG --define gotags=bazel,gss --experimental_proto_descriptor_sets_include_source_info --incompatible_strict_action_env
+build --symlink_prefix=_bazel/ --ui_event_filters=-DEBUG --define gotags=bazel,gss --experimental_proto_descriptor_sets_include_source_info --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution
 test --config=test
 build:with_ui --define cockroach_with_ui=y
 build:test --define crdb_test=y
@@ -20,26 +20,19 @@ test:ci --test_output=errors
 test:ci --test_tmpdir=/artifacts/tmp
 
 build:cross --stamp
-build:cross --host_crosstool_top=@toolchain_cross_x86_64-unknown-linux-gnu//:suite
 build:cross --define cockroach_cross=y
 
-# cross-compilation configurations. Add e.g. --config=crosslinux to turn these on
-# TODO(ricky): Having to specify both the `platform` and the `crosstool_top` is
-# weird, but I think `rules_foreign_cc` doesn't play too nicely with `--platforms`?
+# cross-compilation configurations. Add e.g. --config=crosslinux to turn these on.
 build:crosslinux --platforms=//build/toolchains:cross_linux
-build:crosslinux --crosstool_top=@toolchain_cross_x86_64-unknown-linux-gnu//:suite
 build:crosslinux '--workspace_status_command=./build/bazelutil/stamp.sh x86_64-pc-linux-gnu'
 build:crosslinux --config=cross
 build:crosswindows --platforms=//build/toolchains:cross_windows
-build:crosswindows --crosstool_top=@toolchain_cross_x86_64-w64-mingw32//:suite
 build:crosswindows '--workspace_status_command=./build/bazelutil/stamp.sh x86_64-w64-mingw32'
 build:crosswindows --config=cross
 build:crossmacos --platforms=//build/toolchains:cross_macos
-build:crossmacos --crosstool_top=@toolchain_cross_x86_64-apple-darwin19//:suite
 build:crossmacos '--workspace_status_command=./build/bazelutil/stamp.sh x86_64-apple-darwin19'
 build:crossmacos --config=cross
 build:crosslinuxarm --platforms=//build/toolchains:cross_linux_arm
-build:crosslinuxarm --crosstool_top=@toolchain_cross_aarch64-unknown-linux-gnu//:suite
 build:crosslinuxarm '--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu'
 build:crosslinuxarm --config=cross
 
diff --git a/WORKSPACE b/WORKSPACE
index 08ce59e0ede4..99aa1a22ceed 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -338,10 +338,11 @@ c_deps()
 # aforementioned PRs.
 http_archive(
     name = "rules_foreign_cc",
-    sha256 = "45d48c71f1b1eadc33a5ad15bbeca8ce42f9ef5ab5d7ee519fc4991e6a9e93d2",
-    strip_prefix = "cockroachdb-rules_foreign_cc-f1cff45",
+    sha256 = "272ac2cde4efd316c8d7c0140dee411c89da104466701ac179286ef5a89c7b58",
+    strip_prefix = "cockroachdb-rules_foreign_cc-6f7f1b1",
     urls = [
-        "https://storage.googleapis.com/public-bazel-artifacts/bazel/cockroachdb-rules_foreign_cc-master20210730-1-gf1cff45.zip",
+        # As of commit 6f7f1b1c6f911db5706c2fcbb3d5669d95974a34 (release 0.7.0 plus a couple patches)
+        "https://storage.googleapis.com/public-bazel-artifacts/bazel/cockroachdb-rules_foreign_cc-6f7f1b1.tar.gz",
     ],
 )
 
diff --git a/c-deps/BUILD.bazel b/c-deps/BUILD.bazel
index 14bbf7f17b6a..802a597fb4a2 100644
--- a/c-deps/BUILD.bazel
+++ b/c-deps/BUILD.bazel
@@ -6,10 +6,6 @@ load("@rules_foreign_cc//foreign_cc:configure.bzl", "configure_make")
 configure_make(
     name = "libjemalloc",
     autoconf = True,
-    configure_env_vars = select({
-        "//build/toolchains:dev": {"AR": ""},
-        "//conditions:default": {},
-    }),
     configure_in_place = True,
     configure_options = [
         "--enable-prof",
@@ -19,20 +15,20 @@ configure_make(
         "@io_bazel_rules_go//go/platform:linux_arm64": ["--host=aarch64-unknown-linux-gnu"],
         "//conditions:default": [],
     }),
+    env = select({
+        "//build/toolchains:dev": {"AR": ""},
+        "//conditions:default": {},
+    }),
     lib_source = "@jemalloc//:all",
-    make_commands = [
-        "make build_lib_static",
-        "mkdir -p libjemalloc/lib",
-    ] + select({
-        "@io_bazel_rules_go//go/platform:windows": ["cp lib/jemalloc.lib libjemalloc/lib"],
-        "//conditions:default": ["cp lib/libjemalloc.a libjemalloc/lib"],
-    }) + [
-        "cp -r include libjemalloc",
-    ],
     out_static_libs = select({
         "@io_bazel_rules_go//go/platform:windows": ["jemalloc.lib"],
         "//conditions:default": ["libjemalloc.a"],
     }),
+    targets = [
+        "build_lib_static",
+        "install_lib",
+        "install_include",
+    ],
     visibility = ["//visibility:public"],
 )
 
@@ -58,9 +54,7 @@ cmake(
             "CMAKE_BUILD_TYPE": "Release",
         },
     }),
-    # As of this writing (2021-05-05), foreign_cc
-    # only knows about windows, darwin and linux.
-    cmake_options = ["-GUnix Makefiles"],
+    generate_args = ["-GUnix Makefiles"],
     lib_source = "@proj//:all",
     out_static_libs = ["libproj.a"],
     visibility = ["//visibility:public"],
@@ -88,7 +82,6 @@ cmake(
             "CMAKE_CXX_FLAGS": "-fPIC",
         },
     }),
-    cmake_options = ["-GUnix Makefiles"],
     data = select({
         "//build/toolchains:is_cross_macos": [
             "@toolchain_cross_x86_64-apple-darwin19//:bin/x86_64-apple-darwin19-install_name_tool",
@@ -103,39 +96,11 @@ cmake(
         },
         "//conditions:default": {},
     }),
+    generate_args = ["-GUnix Makefiles"],
     lib_source = "@geos//:all",
-    make_commands = [
-        "mkdir -p libgeos/lib",
-        "make --no-print-directory geos_c",
-    ] + select({
-        "//build/toolchains:is_cross_macos": [
-            "cp -L lib/libgeos.dylib libgeos/lib",
-            "cp -L lib/libgeos_c.dylib libgeos/lib",
-            "PREFIX=$($OTOOL -D lib/libgeos_c.dylib | tail -n1 | rev | cut -d/ -f2- | rev)",
-            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos.3.8.1.dylib libgeos/lib/libgeos.dylib",
-            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos_c.1.dylib libgeos/lib/libgeos_c.dylib",
-            "$CMAKE_INSTALL_NAME_TOOL -change $PREFIX/libgeos.3.8.1.dylib @rpath/libgeos.3.8.1.dylib libgeos/lib/libgeos_c.dylib",
-        ],
-        "@io_bazel_rules_go//go/platform:darwin": [
-            "cp -L lib/libgeos.dylib libgeos/lib",
-            "cp -L lib/libgeos_c.dylib libgeos/lib",
-        ],
-        "@io_bazel_rules_go//go/platform:windows": [
-            # NOTE: Windows ends up in bin/ on the BUILD_TMPDIR.
-            "cp -L bin/libgeos.dll libgeos/lib",
-            "cp -L bin/libgeos_c.dll libgeos/lib",
-        ],
-        "//build/toolchains:is_cross_linux": [
-            "cp -L lib/libgeos.so libgeos/lib",
-            "cp -L lib/libgeos_c.so libgeos/lib",
-            "patchelf --set-rpath /usr/local/lib/cockroach/ libgeos/lib/libgeos_c.so",
-            "patchelf --set-soname libgeos.so libgeos/lib/libgeos.so",
-            "patchelf --replace-needed libgeos.so.3.8.1 libgeos.so libgeos/lib/libgeos_c.so",
-        ],
-        "//conditions:default": [
-            "cp -L lib/libgeos.so libgeos/lib",
-            "cp -L lib/libgeos_c.so libgeos/lib",
-        ],
+    out_lib_dir = select({
+        "@io_bazel_rules_go//go/platform:windows": "bin",
+        "//conditions:default": "lib",
     }),
     out_shared_libs = select({
         "@io_bazel_rules_go//go/platform:darwin": [
@@ -151,6 +116,25 @@ cmake(
             "libgeos.so",
         ],
     }),
+    postfix_script = "mkdir -p libgeos/lib\n" + select({
+        "//build/toolchains:is_cross_macos": (
+            "cp -L lib/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos.dylib\n" +
+            "PREFIX=$($OTOOL -D $INSTALLDIR/lib/libgeos_c.dylib | tail -n1 | rev | cut -d/ -f2- | rev)\n" +
+            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos.dylib\n" +
+            "$CMAKE_INSTALL_NAME_TOOL -id @rpath/libgeos_c.1.dylib $INSTALLDIR/lib/libgeos_c.dylib\n" +
+            "$CMAKE_INSTALL_NAME_TOOL -change $PREFIX/libgeos.3.8.1.dylib @rpath/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos_c.dylib\n"
+        ),
+        "@io_bazel_rules_go//go/platform:darwin": "cp -L lib/libgeos.3.8.1.dylib $INSTALLDIR/lib/libgeos.dylib",
+        "@io_bazel_rules_go//go/platform:windows": "",
+        "//build/toolchains:is_cross_linux": (
+            "cp -L lib/libgeos.so.3.8.1 $INSTALLDIR/lib/libgeos.so\n" +
+            "patchelf --set-rpath /usr/local/lib/cockroach/ $INSTALLDIR/lib/libgeos_c.so\n" +
+            "patchelf --set-soname libgeos.so $INSTALLDIR/lib/libgeos.so\n" +
+            "patchelf --replace-needed libgeos.so.3.8.1 libgeos.so $INSTALLDIR/lib/libgeos_c.so\n"
+        ),
+        "//conditions:default": "cp -L lib/libgeos.so.3.8.1 $INSTALLDIR/lib/libgeos.so",
+    }),
+    targets = ["geos_c"],
     visibility = ["//visibility:public"],
 )
 
@@ -165,22 +149,13 @@ configure_make(
         "--enable-static",
         "--disable-shared",
     ],
+    # We specify -fcommon to get around duplicate definition errors in recent gcc.
+    copts = ["-fcommon"],
     data = [":autom4te"],
     env = {
         "AUTOM4TE": "$(execpath :autom4te)",
     },
     lib_source = "@krb5//:all",
-    make_commands = [
-        "make",
-        "mkdir -p libkrb5/lib",
-        "cp lib/libcom_err.a libkrb5/lib",
-        "cp lib/libgssapi_krb5.a libkrb5/lib",
-        "cp lib/libkrb5.a libkrb5/lib",
-        "cp lib/libkrb5support.a libkrb5/lib",
-        "cp lib/libk5crypto.a libkrb5/lib",
-        "mkdir -p libkrb5/include/gssapi",
-        "cp include/gssapi/gssapi.h libkrb5/include/gssapi",
-    ],
     out_static_libs = [
         "libgssapi_krb5.a",
         "libkrb5.a",
@@ -188,6 +163,14 @@ configure_make(
         "libk5crypto.a",
         "libcom_err.a",
     ],
+    postfix_script = ("""mkdir -p libkrb5/lib
+cp lib/libcom_err.a libkrb5/lib
+cp lib/libgssapi_krb5.a libkrb5/lib
+cp lib/libkrb5.a libkrb5/lib
+cp lib/libkrb5support.a libkrb5/lib
+cp lib/libk5crypto.a libkrb5/lib
+mkdir -p libkrb5/include/gssapi
+cp include/gssapi/gssapi.h libkrb5/include/gssapi"""),
     visibility = ["//visibility:public"],
 )