Skip to content

Commit

Permalink
tree: skip nulls when type checking a list of tuples
Browse files Browse the repository at this point in the history
Previously, the type checking code for a list of tuples expected that
all expressions in the list are actually tuples. Now we will also allow
for nulls to be present in the list.

Release note (sql change): NULLs are now allowed to be among tuples when
being compared against a tuple.
  • Loading branch information
yuzefovich committed Oct 15, 2019
1 parent a40cefb commit 7fe1b19
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 19 deletions.
23 changes: 23 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/builtin_function
Expand Up @@ -2327,3 +2327,26 @@ query TT
SELECT pg_get_function_result('pg_sleep'::regproc), pg_get_function_result('unnest'::regproc)
----
bool anyelement

# Regression test for #40297.
statement ok
CREATE TABLE t40297 AS SELECT g FROM generate_series(NULL, NULL) AS g

query I
SELECT COALESCE((SELECT ()), NULL) FROM t40297
----

query T
SELECT CASE WHEN true THEN (1, 2) ELSE NULL END
----
(1,2)

query B
SELECT (1, 2) IN ((2, 3), NULL, (1, 2))
----
true

query B
SELECT (1, 2) IN ((2, 3), NULL)
----
NULL
14 changes: 10 additions & 4 deletions pkg/sql/sem/tree/eval.go
Expand Up @@ -2316,11 +2316,17 @@ func makeEvalTupleIn(typ *types.T) *CmpOp {
} else {
// The left-hand side is a tuple, e.g. `(1, 2) IN ((1, 2), (3, 4))`.
for _, val := range vtuple.D {
// Use the EQ function which properly handles NULLs.
if res := cmpOpTupleFn(ctx, *argTuple, *val.(*DTuple), EQ); res == DNull {
if val == DNull {
// We allow for a null value to be in the list of tuples, so we
// need to check that upfront.
sawNull = true
} else if res == DBoolTrue {
return DBoolTrue, nil
} else {
// Use the EQ function which properly handles NULLs.
if res := cmpOpTupleFn(ctx, *argTuple, *val.(*DTuple), EQ); res == DNull {
sawNull = true
} else if res == DBoolTrue {
return DBoolTrue, nil
}
}
}
}
Expand Down
52 changes: 37 additions & 15 deletions pkg/sql/sem/tree/type_check.go
Expand Up @@ -2013,12 +2013,13 @@ func typeCheckTupleComparison(
return left, right, nil
}

// typeCheckSameTypedTupleExprs type checks a list of expressions, asserting that all
// are tuples which have the same type. The function expects the first provided expression
// to be a tuple, and will panic if it is not. However, it does not expect all other
// expressions are tuples, and will return a sane error if they are not. An optional
// desired type can be provided, which will hint that type which the expressions should
// resolve to, if possible.
// typeCheckSameTypedTupleExprs type checks a list of expressions, asserting
// that all are tuples which have the same type or nulls. The function expects
// the first provided expression to be a tuple, and will panic if it is not.
// However, it does not expect all other expressions are tuples or nulls, and
// will return a sane error if they are not. An optional desired type can be
// provided, which will hint that type which the expressions should resolve to,
// if possible.
func typeCheckSameTypedTupleExprs(
ctx *SemaContext, desired *types.T, exprs ...Expr,
) ([]TypedExpr, *types.T, error) {
Expand All @@ -2028,7 +2029,7 @@ func typeCheckSameTypedTupleExprs(

// All other exprs must be tuples.
first := exprs[0].(*Tuple)
if err := checkAllExprsAreTuples(ctx, exprs[1:]); err != nil {
if err := checkAllExprsAreTuplesOrNulls(ctx, exprs[1:]); err != nil {
return nil, nil, err
}

Expand All @@ -2038,12 +2039,21 @@ func typeCheckSameTypedTupleExprs(
return nil, nil, err
}

// All expressions at the same indexes must be the same type.
// All expressions within tuples at the same indexes must be the same type.
resTypes := types.MakeTuple(make([]types.T, firstLen))
sameTypeExprs := make([]Expr, len(exprs))
sameTypeExprs := make([]Expr, 0, len(exprs))
// We will be skipping nulls, so we need to keep track at which indices in
// exprs are the non-null tuples.
sameTypeExprsIndices := make([]int, 0, len(exprs))
for elemIdx := range first.Exprs {
for tupleIdx, expr := range exprs {
sameTypeExprs[tupleIdx] = expr.(*Tuple).Exprs[elemIdx]
sameTypeExprs = sameTypeExprs[:0]
sameTypeExprsIndices = sameTypeExprsIndices[:0]
for exprIdx, expr := range exprs {
if expr == DNull {
continue
}
sameTypeExprs = append(sameTypeExprs, expr.(*Tuple).Exprs[elemIdx])
sameTypeExprsIndices = append(sameTypeExprsIndices, exprIdx)
}
desiredElem := types.Any
if len(desired.TupleContents()) > elemIdx {
Expand All @@ -2054,20 +2064,27 @@ func typeCheckSameTypedTupleExprs(
return nil, nil, pgerror.Newf(pgcode.DatatypeMismatch, "tuples %s are not the same type: %v", Exprs(exprs), err)
}
for j, typedExpr := range typedSubExprs {
exprs[j].(*Tuple).Exprs[elemIdx] = typedExpr
tupleIdx := sameTypeExprsIndices[j]
exprs[tupleIdx].(*Tuple).Exprs[elemIdx] = typedExpr
}
resTypes.TupleContents()[elemIdx] = *resType
}
for tupleIdx, expr := range exprs {
expr.(*Tuple).typ = resTypes
if expr != DNull {
expr.(*Tuple).typ = resTypes
}
typedExprs[tupleIdx] = expr.(TypedExpr)
}
return typedExprs, resTypes, nil
}

func checkAllExprsAreTuples(ctx *SemaContext, exprs []Expr) error {
// checkAllExprsAreTuplesOrNulls checks that all expressions in exprs are
// either tuples or nulls.
func checkAllExprsAreTuplesOrNulls(ctx *SemaContext, exprs []Expr) error {
for _, expr := range exprs {
if _, ok := expr.(*Tuple); !ok {
_, isTuple := expr.(*Tuple)
isNull := expr == DNull
if !(isTuple || isNull) {
typedExpr, err := expr.TypeCheck(ctx, types.Any)
if err != nil {
return err
Expand All @@ -2078,8 +2095,13 @@ func checkAllExprsAreTuples(ctx *SemaContext, exprs []Expr) error {
return nil
}

// checkAllTuplesHaveLength checks that all tuples in exprs have the expected
// length. Note that all nulls are skipped in this check.
func checkAllTuplesHaveLength(exprs []Expr, expectedLen int) error {
for _, expr := range exprs {
if expr == DNull {
continue
}
if err := checkTupleHasLength(expr.(*Tuple), expectedLen); err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/tree/type_check_internal_test.go
Expand Up @@ -292,6 +292,7 @@ func TestTypeCheckSameTypedTupleExprs(t *testing.T) {
// Verify dealing with Null.
{nil, nil, exprs(tuple(intConst("1"), dnull), tuple(dnull, decConst("1"))), ttuple(types.Int, types.Decimal), nil},
{nil, nil, exprs(tuple(dint(1), dnull), tuple(dnull, ddecimal(1))), ttuple(types.Int, types.Decimal), nil},
{nil, nil, exprs(tuple(dint(1), dnull), dnull, tuple(dint(1), dnull), dnull), ttuple(types.Int, types.Unknown), nil},
// Verify desired type when possible.
{nil, ttuple(types.Int, types.Decimal), exprs(tuple(intConst("1"), intConst("1")), tuple(intConst("1"), intConst("1"))), ttuple(types.Int, types.Decimal), nil},
// Verify desired type when possible with unresolved constants.
Expand Down

0 comments on commit 7fe1b19

Please sign in to comment.