Skip to content

Commit bf7e23f

Browse files
craig[bot]cucaroach
andcommitted
108199: sql: fix overload type checking of nested case expressions r=mgartner a=cucaroach If we have a nested case expression where the inner case expression is ambiguous the AnyCollatedString type would be selected and it would leak to the execution engine causing the 'failed to parse locale ""' internal error. Instead have the overload type checker remember if it saw a AnyCollatedString type and go back and repair types if a concrete type is found in any of the other exprs. Release note: none Epic: none Fixes: cockroachdb#101418 Co-authored-by: Tommy Reilly <[email protected]>
2 parents 8e614fc + 6038c50 commit bf7e23f

File tree

2 files changed

+59
-0
lines changed

2 files changed

+59
-0
lines changed

pkg/sql/sem/tree/overload.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,7 @@ func (s *overloadTypeChecker) typeCheckOverloadedExprs(
810810

811811
// Filter out overloads on resolved types. This includes resolved placeholders
812812
// and any other resolvable exprs.
813+
ambiguousCollatedTypes := false
813814
var typeableIdxs intsets.Fast
814815
for i, ok := s.resolvableIdxs.Next(0); ok; i, ok = s.resolvableIdxs.Next(i + 1) {
815816
typeableIdxs.Add(i)
@@ -836,6 +837,8 @@ func (s *overloadTypeChecker) typeCheckOverloadedExprs(
836837
break
837838
}
838839
}
840+
// Don't allow ambiguous types to be desired, this prevents for instance
841+
// AnyCollatedString from trumping the concrete collated type.
839842
if sameType != nil {
840843
paramDesired = sameType
841844
}
@@ -844,6 +847,9 @@ func (s *overloadTypeChecker) typeCheckOverloadedExprs(
844847
return err
845848
}
846849
s.typedExprs[i] = typ
850+
if typ.ResolvedType() == types.AnyCollatedString {
851+
ambiguousCollatedTypes = true
852+
}
847853
rt := typ.ResolvedType()
848854
s.overloadIdxs = filterParams(s.overloadIdxs, s.params, func(
849855
params TypeList,
@@ -852,6 +858,32 @@ func (s *overloadTypeChecker) typeCheckOverloadedExprs(
852858
})
853859
}
854860

861+
// If we typed any exprs as AnyCollatedString but have a concrete collated
862+
// string type, redo the types using the contrete type. Note we're probably
863+
// still lacking full compliance with PG on collation handling:
864+
// https://www.postgresql.org/docs/current/collation.html#id-1.6.11.4.4
865+
if ambiguousCollatedTypes {
866+
var concreteType *types.T
867+
for i, ok := typeableIdxs.Next(0); ok; i, ok = typeableIdxs.Next(i + 1) {
868+
typ := s.typedExprs[i].ResolvedType()
869+
if typ != types.AnyCollatedString {
870+
concreteType = typ
871+
break
872+
}
873+
}
874+
if concreteType != nil {
875+
for i, ok := typeableIdxs.Next(0); ok; i, ok = typeableIdxs.Next(i + 1) {
876+
if s.typedExprs[i].ResolvedType() == types.AnyCollatedString {
877+
typ, err := s.exprs[i].TypeCheck(ctx, semaCtx, concreteType)
878+
if err != nil {
879+
return err
880+
}
881+
s.typedExprs[i] = typ
882+
}
883+
}
884+
}
885+
}
886+
855887
// At this point, all remaining overload candidates accept the argument list,
856888
// so we begin checking for a single remainig candidate implementation to choose.
857889
// In case there is more than one candidate remaining, the following code uses

pkg/sql/sem/tree/type_check_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,33 @@ func TestTypeCheckCollatedString(t *testing.T) {
473473
require.Equal(t, rightTyp.Locale(), "en-US-u-ks-level2")
474474
}
475475

476+
func TestTypeCheckCollatedStringNestedCaseComparison(t *testing.T) {
477+
defer leaktest.AfterTest(t)()
478+
defer log.Scope(t).Close(t)
479+
480+
ctx := context.Background()
481+
semaCtx := tree.MakeSemaContext()
482+
483+
// The collated string constant must be on the LHS for this test, so that
484+
// the type-checker chooses the collated string overload first.
485+
for _, exprStr := range []string{
486+
`CASE WHEN false THEN CASE WHEN (NOT (false)) THEN NULL END ELSE ('' COLLATE "es_ES") END >= ('' COLLATE "es_ES")`,
487+
`CASE WHEN false THEN NULL ELSE ('' COLLATE "es_ES") END >= ('' COLLATE "es_ES")`,
488+
`CASE WHEN false THEN ('' COLLATE "es_ES") ELSE NULL END >= ('' COLLATE "es_ES")`,
489+
`('' COLLATE "es_ES") >= CASE WHEN false THEN CASE WHEN (NOT (false)) THEN NULL END ELSE ('' COLLATE "es_ES") END`} {
490+
expr, err := parser.ParseExpr(exprStr)
491+
require.NoError(t, err)
492+
typed, err := tree.TypeCheck(ctx, expr, &semaCtx, types.Any)
493+
require.NoError(t, err)
494+
495+
for _, ex := range []tree.Expr{typed.(*tree.ComparisonExpr).Left, typed.(*tree.ComparisonExpr).Right} {
496+
typ := ex.(tree.TypedExpr).ResolvedType()
497+
require.Equal(t, types.CollatedStringFamily, typ.Family())
498+
require.Equal(t, "es_ES", typ.Locale())
499+
}
500+
}
501+
}
502+
476503
func TestTypeCheckCaseExprWithPlaceholders(t *testing.T) {
477504
defer leaktest.AfterTest(t)()
478505
defer log.Scope(t).Close(t)

0 commit comments

Comments
 (0)