From c725fabd19fe6fbb927cb208d30943d73d9a1447 Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Mon, 7 Feb 2022 14:31:14 -0500 Subject: [PATCH] optbuilder: do not create invalid casts when building CASE expressions The optbuilder no longer creates invalid casts when building CASE expressions that have branches of different types. CASE expressions that previously caused internal errors now result in user-facing errors. A similar change was made recently to UNION expressions in #75219. Note that there is more work to be done to be fully consistent with Postgres's CASE typing behavior, see #75365. Fixes #75365 Release note (bug fix): CASE expressions with branches that result in types that cannot be cast to a common type now result in a user-facing error instead of an internal error. --- pkg/sql/opt/optbuilder/scalar.go | 29 ++++++++++++++++++++++++-- pkg/sql/opt/optbuilder/testdata/scalar | 15 +++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/pkg/sql/opt/optbuilder/scalar.go b/pkg/sql/opt/optbuilder/scalar.go index 67c7a0b7a5e9..1c3fbe79bd57 100644 --- a/pkg/sql/opt/optbuilder/scalar.go +++ b/pkg/sql/opt/optbuilder/scalar.go @@ -233,18 +233,43 @@ func (b *Builder) buildScalar( input = memo.TrueSingleton } + // validateCastToValType panics if tree.ReType with the given source + // type would create an invalid cast to valType. + validateCastToValType := func(src *types.T) { + if valType.Family() == types.AnyFamily || src.Identical(valType) { + // If valType's family is AnyFamily or src is identical to + // valType, then tree.Retype will not create a cast expression. + return + } + if tree.ValidCast(src, valType, tree.CastContextExplicit) { + // 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. + return + } + panic(pgerror.Newf( + pgcode.DatatypeMismatch, + "CASE types %s and %s cannot be matched", src, valType, + )) + } + whens := make(memo.ScalarListExpr, 0, len(t.Whens)+1) for i := range t.Whens { condExpr := t.Whens[i].Cond.(tree.TypedExpr) cond := b.buildScalar(condExpr, inScope, nil, nil, colRefs) - valExpr := tree.ReType(t.Whens[i].Val.(tree.TypedExpr), valType) + valExpr := t.Whens[i].Val.(tree.TypedExpr) + validateCastToValType(valExpr.ResolvedType()) + valExpr = tree.ReType(valExpr, valType) val := b.buildScalar(valExpr, inScope, nil, nil, colRefs) whens = append(whens, b.factory.ConstructWhen(cond, val)) } // Add the ELSE expression to the end of whens as a raw scalar expression. var orElse opt.ScalarExpr if t.Else != nil { - elseExpr := tree.ReType(t.Else.(tree.TypedExpr), valType) + elseExpr := t.Else.(tree.TypedExpr) + validateCastToValType(elseExpr.ResolvedType()) + elseExpr = tree.ReType(elseExpr, valType) orElse = b.buildScalar(elseExpr, inScope, nil, nil, colRefs) } else { orElse = b.factory.ConstructNull(valType) diff --git a/pkg/sql/opt/optbuilder/testdata/scalar b/pkg/sql/opt/optbuilder/testdata/scalar index 149857efb281..231a462d813d 100644 --- a/pkg/sql/opt/optbuilder/testdata/scalar +++ b/pkg/sql/opt/optbuilder/testdata/scalar @@ -1485,3 +1485,18 @@ is [type=bool] │ │ └── null [type=unknown] │ └── array: [type=string[]] └── null [type=unknown] + +# Regression test for #75365. Do not create invalid casts when building CASE +# expressions. We build a Select expressions here instead of a scalar so that +# logical properties are generated, which is required to reproduce the bug. +# TODO(#75103): We should be more permissive with casts of arrays of tuples. +# These tests should be successful, not user-facing errors. +build +SELECT CASE WHEN false THEN ARRAY[('', 0)] ELSE ARRAY[]::RECORD[] END +---- +error (42804): CASE types tuple[] and tuple{string, int}[] cannot be matched + +build +SELECT CASE WHEN false THEN ARRAY[('', 0)] WHEN true THEN ARRAY[]::RECORD[] ELSE ARRAY[('', 0)] END +---- +error (42804): CASE types tuple[] and tuple{string, int}[] cannot be matched