diff --git a/pkg/cmd/roachtest/tests/sqlsmith.go b/pkg/cmd/roachtest/tests/sqlsmith.go index 2b3b0eb35ab6..24c3767b8ac6 100644 --- a/pkg/cmd/roachtest/tests/sqlsmith.go +++ b/pkg/cmd/roachtest/tests/sqlsmith.go @@ -246,11 +246,10 @@ INSERT INTO seed_mr_table DEFAULT VALUES;`, regionList[0]), es := err.Error() if strings.Contains(es, "internal error") { // TODO(yuzefovich): we temporarily ignore internal errors - // that are because of #40929 and #70831. + // that are because of #40929. var expectedError bool for _, exp := range []string{ "could not parse \"0E-2019\" as type decimal", - "no volatility for cast tuple", } { expectedError = expectedError || strings.Contains(es, exp) } 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 25effe7a3db0..38d9b25aa8d9 100644 --- a/pkg/sql/opt/optbuilder/union.go +++ b/pkg/sql/opt/optbuilder/union.go @@ -153,15 +153,24 @@ 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 !tree.ValidCast(src, tgt, tree.CastContextExplicit) { + // Error if no cast exists from src to tgt. + // TODO(#75103): For legacy reasons, we check for a valid cast in + // the most permissive context, CastContextExplicit. To be + // consistent with Postgres, we should check for a valid cast in the + // most restrictive context, CastContextImplicit. + 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 +198,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 +207,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 dstCols := dst.cols diff --git a/pkg/sql/opt/optbuilder/union_test.go b/pkg/sql/opt/optbuilder/union_test.go index 112d5159e3a0..557bd319b6bf 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.Any})), + right: types.MakeArray(types.MakeTuple([]*types.T{types.Bool})), + expected: nil, + }, } for _, tc := range testCases {