From 91194f153a0b255399e2429bbb2e47f3ab02955e Mon Sep 17 00:00:00 2001
From: Marcus Gartner <marcus@cockroachlabs.com>
Date: Thu, 20 Jan 2022 15:43:26 +0000
Subject: [PATCH] opt: do not add invalid casts to match types in set
 operations

The optbuilder adds casts in order to match non-identical left/right
types of set operations like UNION (see #60560). This change prevents
the optbuilder from adding casts that are invalid, which caused internal
errors. Now, a user error is thrown if no such cast exists from the left
or right input type to the output type.

Release note (bug fix): A bug has been fixed that caused internal errors
in queries with set operations, like UNION, when corresponding columns
on either side of the set operation were not the same. This error only
occurred with a limited set of types. This bug is present in versions
20.2.6+, 21.1.0+, and 21.2.0+.
---
 pkg/sql/opt/optbuilder/testdata/union | 14 ++++++++++++++
 pkg/sql/opt/optbuilder/union.go       | 25 ++++++++++++++++---------
 pkg/sql/opt/optbuilder/union_test.go  |  6 ++++++
 3 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/pkg/sql/opt/optbuilder/testdata/union b/pkg/sql/opt/optbuilder/testdata/union
index 8da5e9163cd1..9625f180e023 100644
--- a/pkg/sql/opt/optbuilder/testdata/union
+++ b/pkg/sql/opt/optbuilder/testdata/union
@@ -1079,3 +1079,17 @@ sort
            ├── columns: t68702.a:7 b:8 c:9
            └── scan t68702
                 └── columns: t68702.a:7 b:8 c:9 rowid:10!null crdb_internal_mvcc_timestamp:11 tableoid:12
+
+
+# Regression test for #70831. Do not add non-existent casts to match types on
+# either side of a set operation.
+# NOTE: In Postgres this example successfully returns a result. Our tuple typing
+# and casting logic doesn't match Postgres's semantics exactly, so we return a
+# user error. See
+# https://github.com/cockroachdb/cockroach/issues/70831#issuecomment-929547297.
+build
+SELECT * FROM (VALUES (ARRAY[(true, NULL)])) AS v1
+EXCEPT ALL
+SELECT * FROM (VALUES (ARRAY[]::RECORD[])) AS v2
+----
+error (42804): EXCEPT types tuple{bool, unknown}[] and tuple[] cannot be matched
diff --git a/pkg/sql/opt/optbuilder/union.go b/pkg/sql/opt/optbuilder/union.go
index 4dcfb27da631..f34373096d28 100644
--- a/pkg/sql/opt/optbuilder/union.go
+++ b/pkg/sql/opt/optbuilder/union.go
@@ -153,15 +153,20 @@ func determineUnionType(left, right *types.T, clauseTag string) *types.T {
 	}
 
 	if left.Equivalent(right) {
-		// Do a best-effort attempt to determine which type is "larger".
-		if left.Width() > right.Width() {
-			return left
-		}
+		// In the default case, use the left type.
+		src, tgt := right, left
 		if left.Width() < right.Width() {
-			return right
+			// If the right type is "larger", use it.
+			src, tgt = left, right
 		}
-		// In other cases, use the left type.
-		return left
+		if _, ok := tree.LookupCastVolatility(src, tgt, nil /* sessionData */); !ok {
+			// Error if no cast exists from src to tgt.
+			panic(pgerror.Newf(
+				pgcode.DatatypeMismatch,
+				"%v types %s and %s cannot be matched", clauseTag, left, right,
+			))
+		}
+		return tgt
 	}
 	leftFam, rightFam := left.Family(), right.Family()
 
@@ -189,7 +194,7 @@ func determineUnionType(left, right *types.T, clauseTag string) *types.T {
 		return right
 	}
 
-	// TODO(radu): Postgres has more encompassing rules:
+	// TODO(#75103): Postgres has more encompassing rules:
 	// http://www.postgresql.org/docs/12/static/typeconv-union-case.html
 	panic(pgerror.Newf(
 		pgcode.DatatypeMismatch,
@@ -198,7 +203,9 @@ func determineUnionType(left, right *types.T, clauseTag string) *types.T {
 }
 
 // addCasts adds a projection to a scope, adding casts as necessary so that the
-// resulting columns have the given types.
+// resulting columns have the given types. This function assumes that there is a
+// valid cast from the column types in dst.cols to the corresponding types in
+// outTypes.
 func (b *Builder) addCasts(dst *scope, outTypes []*types.T) *scope {
 	expr := dst.expr.(memo.RelExpr)
 	dstCols := dst.cols
diff --git a/pkg/sql/opt/optbuilder/union_test.go b/pkg/sql/opt/optbuilder/union_test.go
index 112d5159e3a0..6f04e16e98f2 100644
--- a/pkg/sql/opt/optbuilder/union_test.go
+++ b/pkg/sql/opt/optbuilder/union_test.go
@@ -72,6 +72,12 @@ func TestUnionType(t *testing.T) {
 			right:    types.String,
 			expected: nil,
 		},
+		{
+			// Error.
+			left:     types.MakeArray(types.MakeTuple([]*types.T{types.Bool})),
+			right:    types.MakeArray(types.MakeTuple([]*types.T{types.Bool, types.Int})),
+			expected: nil,
+		},
 	}
 
 	for _, tc := range testCases {