Skip to content

Commit

Permalink
Merge #75219
Browse files Browse the repository at this point in the history
75219: opt: do not add invalid casts to match types in set operations r=mgartner a=mgartner

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.

Fixes #70831

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+.

Co-authored-by: Marcus Gartner <[email protected]>
  • Loading branch information
craig[bot] and mgartner committed Jan 21, 2022
2 parents 61fa0d5 + f46f377 commit 28c24d6
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 11 deletions.
3 changes: 1 addition & 2 deletions pkg/cmd/roachtest/tests/sqlsmith.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/union
Original file line number Diff line number Diff line change
Expand Up @@ -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
29 changes: 20 additions & 9 deletions pkg/sql/opt/optbuilder/union.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/opt/optbuilder/union_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 28c24d6

Please sign in to comment.