Skip to content

Commit

Permalink
tree: don't resolve expressions with wildcard types
Browse files Browse the repository at this point in the history
There are various wildcard types that are used during type-checking,
but which are not valid during execution. Previously, we only checked
for `types.Any` in several places, but there are other wildcard types
like `types.AnyEnum` with similar properties. This could lead to an
internal panic during execution when this invalid type was resolved.
This patch adds a new method `types.T.IsWildcardType()` that checks
for all wildcard types, to be used during type-checking.

Fixes #83496

Release note (bug fix): Fixed a bug that has existed since before v22.2
which could cause an internal error during distributed execution for an
expression like `CASE` that requires its inputs to be the same type
with all `NULL` inputs.
  • Loading branch information
DrewKimball committed Aug 17, 2023
1 parent 673bde5 commit 9f7a25a
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 5 deletions.
15 changes: 15 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/typing
Original file line number Diff line number Diff line change
Expand Up @@ -273,3 +273,18 @@ SELECT ARRAY[]::TIMESTAMPTZ[] >
query error pq: cannot determine type of empty array\. Consider casting to the desired type, for example ARRAY\[\]::int\[\]
SELECT ARRAY[]::TIMESTAMPTZ[] <
SOME (ARRAY[TIMESTAMPTZ '1969-12-29T21:20:13+01'], ARRAY[NULL]);

# Static analysis types should never make it to execution.
statement ok
CREATE TABLE t83496 (
a STRING,
b STRING
);
CREATE TYPE typ83496 AS ENUM ('foo', 'bar');

statement ok
SELECT * FROM t83496
WHERE (
(SELECT 'bar':::typ83496 FROM t83496 WHERE false)
IS NOT DISTINCT FROM CASE WHEN NULL THEN NULL ELSE NULL END
);
10 changes: 5 additions & 5 deletions pkg/sql/sem/tree/type_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -1706,7 +1706,7 @@ func (expr *Array) TypeCheck(
}

if len(expr.Exprs) == 0 {
if desiredParam.Family() == types.AnyFamily {
if desiredParam.IsWildcardType() {
return nil, errAmbiguousArrayType
}
expr.typ = types.MakeArray(desiredParam)
Expand Down Expand Up @@ -2576,7 +2576,7 @@ func typeCheckSameTypedExprs(
return nil, nil, err
}
typ := typedExpr.ResolvedType()
if typ == types.Unknown && desired != types.Any {
if typ == types.Unknown && !desired.IsWildcardType() {
// The expression had a NULL type, so we can return the desired type as
// the expression type.
typ = desired
Expand Down Expand Up @@ -2642,7 +2642,7 @@ func typeCheckSameTypedExprs(
}
return typedExprs, typ, nil
default:
if desired != types.Any {
if !desired.IsWildcardType() {
return typedExprs, desired, nil
}
return typedExprs, types.Unknown, nil
Expand Down Expand Up @@ -2709,7 +2709,7 @@ func typeCheckSameTypedConsts(
}

// If typ is not a wildcard, all consts try to become typ.
if typ.Family() != types.AnyFamily {
if !typ.IsWildcardType() {
all := true
for i, ok := s.constIdxs.Next(0); ok; i, ok = s.constIdxs.Next(i + 1) {
if !canConstantBecome(s.exprs[i].(Constant), typ) {
Expand Down Expand Up @@ -2748,7 +2748,7 @@ func typeCheckSameTypedConsts(
if typ := typedExpr.ResolvedType(); !typ.Equivalent(reqTyp) {
return nil, unexpectedTypeError(s.exprs[i], reqTyp, typ)
}
if reqTyp.Family() == types.AnyFamily {
if reqTyp.IsWildcardType() {
reqTyp = typedExpr.ResolvedType()
}
}
Expand Down
10 changes: 10 additions & 0 deletions pkg/sql/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2102,6 +2102,16 @@ func (t *T) Equal(other *T) bool {
return t.Identical(other)
}

// IsWildcardType returns true if the type is only used as a wildcard during
// static analysis, and cannot be used during execution.
func (t *T) IsWildcardType() bool {
switch t {
case Any, AnyArray, AnyCollatedString, AnyEnum, AnyEnumArray, AnyTuple, AnyTupleArray:
return true
}
return false
}

// Size returns the size, in bytes, of this type once it has been marshaled to
// a byte buffer. This is typically called to determine the size of the buffer
// that needs to be allocated before calling Marshal.
Expand Down

0 comments on commit 9f7a25a

Please sign in to comment.