diff --git a/pkg/sql/logictest/testdata/logic_test/builtin_function b/pkg/sql/logictest/testdata/logic_test/builtin_function index c1f8703e0353..e279b7c62211 100644 --- a/pkg/sql/logictest/testdata/logic_test/builtin_function +++ b/pkg/sql/logictest/testdata/logic_test/builtin_function @@ -2572,14 +2572,6 @@ bool query error pq: more than one function named 'unnest' SELECT pg_get_function_result('unnest'::regproc); -# 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 ---- diff --git a/pkg/sql/logictest/testdata/logic_test/record b/pkg/sql/logictest/testdata/logic_test/record index 861ac7a699dd..d0a6224731b3 100644 --- a/pkg/sql/logictest/testdata/logic_test/record +++ b/pkg/sql/logictest/testdata/logic_test/record @@ -180,3 +180,20 @@ SELECT s, s::a FROM strings ORDER BY 1 ---- (1,2) (1,2) (5,6) (5,6) + +query T +SELECT '(1 , 2)'::a +---- +(1," 2") + +statement error pgcode 22P02 malformed record literal +SELECT '()'::a + +statement error pgcode 0A000 cannot parse anonymous record type +SELECT s, s::record FROM strings ORDER BY 1 + +statement error pgcode 0A000 cannot parse anonymous record type +SELECT '()'::record + +statement error pgcode 0A000 cannot parse anonymous record type +SELECT '(1,4)'::record diff --git a/pkg/sql/logictest/testdata/logic_test/tuple b/pkg/sql/logictest/testdata/logic_test/tuple index 9486d76da16d..81fbd2b43c3f 100644 --- a/pkg/sql/logictest/testdata/logic_test/tuple +++ b/pkg/sql/logictest/testdata/logic_test/tuple @@ -980,6 +980,57 @@ SELECT (1::INT, NULL) ---- (1,) +# Regression test for #40297: make sure tuples and nulls type-check when +# used together in a "same-typed expression" such as function arguments. +subtest regression_40297 + +statement ok +CREATE TABLE t(x INT); +CREATE TABLE t40297 AS SELECT g FROM generate_series(NULL, NULL) AS g + +query I +SELECT COALESCE((SELECT ()), NULL) FROM t40297 +---- + +# Adding a cast shouldn't affect the type-checking. +query I +SELECT COALESCE((SELECT ())::record, NULL) FROM t40297 +---- + +# Tuples must be the same length. +statement error incompatible COALESCE expressions: expected \(SELECT \(1,\)\) to be of type tuple, found type tuple{int} +SELECT COALESCE((SELECT ()), (SELECT ROW (1))) FROM t40297 + +# Adding a cast here should still work too. +statement error incompatible COALESCE expressions: expected \(SELECT \(1,\)\)::T to be of type tuple, found type tuple{int AS x} +SELECT COALESCE((SELECT ()), (SELECT ROW (1))::t) FROM t40297 + +# If a NULL is casted, then it no longer can be coerced to the right type. +statement error incompatible COALESCE expressions: expected NULL::T to be of type tuple, found type tuple{int AS x} +SELECT COALESCE((SELECT ()), NULL::t) FROM t40297 + +# Type-checking should still work for non-empty tuples. +query I +SELECT COALESCE((SELECT '(1)'::t), NULL::t) FROM t40297 +---- + +# (SELECT (1)) is interpreted as an INT. +statement error incompatible COALESCE expressions: expected NULL::T to be of type int, found type tuple{int AS x} +SELECT COALESCE((SELECT (1)), NULL::t) FROM t40297 + +# Adding ROW allows it to be interpreted as a tuple. +query I +SELECT COALESCE((SELECT ROW (1)), NULL::t) FROM t40297 +---- + +# An anonymous tuple type only works if it is used with a tuple of the correct length. +statement error incompatible COALESCE expressions: expected NULL::T to be of type tuple{int, int}, found type tuple{int AS x} +SELECT COALESCE((SELECT ROW (1, 2)), NULL::t) FROM t40297 + +# It should still type-check correctly without using a subquery. +statement error incompatible COALESCE expressions: expected NULL::T to be of type tuple{int, int}, found type tuple{int AS x} +SELECT COALESCE(ROW (1, 2), NULL::t) FROM t40297 + # Regression test for #70767. Make sure we avoid encoding arrays where the # array content type is AnyTuple. subtest regression_70767 diff --git a/pkg/sql/opt/exec/execbuilder/scalar.go b/pkg/sql/opt/exec/execbuilder/scalar.go index 292215707e60..89ee63db3145 100644 --- a/pkg/sql/opt/exec/execbuilder/scalar.go +++ b/pkg/sql/opt/exec/execbuilder/scalar.go @@ -112,10 +112,6 @@ func (b *Builder) buildTypedExpr( } func (b *Builder) buildNull(ctx *buildScalarCtx, scalar opt.ScalarExpr) (tree.TypedExpr, error) { - if scalar.DataType().Family() == types.TupleFamily && !scalar.DataType().UserDefined() { - // See comment in buildCast. - return tree.DNull, nil - } return tree.ReType(tree.DNull, scalar.DataType()), nil } @@ -337,15 +333,6 @@ func (b *Builder) buildCast(ctx *buildScalarCtx, scalar opt.ScalarExpr) (tree.Ty if err != nil { return nil, err } - if cast.Typ.Family() == types.TupleFamily && !cast.Typ.UserDefined() { - // TODO(radu): casts to Tuple are not supported (they can't be serialized - // for distsql) unless they are user-defined, in which case they have an - // OID and can be serialized with the ::@ syntax. This should only - // happen when the input is always NULL so the expression should still be - // valid without the cast (though there could be cornercases where the type - // does matter). - return input, nil - } return tree.NewTypedCastExpr(input, cast.Typ), nil } diff --git a/pkg/sql/sem/tree/cast.go b/pkg/sql/sem/tree/cast.go index 7495167f5ac7..6be3ef932d28 100644 --- a/pkg/sql/sem/tree/cast.go +++ b/pkg/sql/sem/tree/cast.go @@ -2309,6 +2309,10 @@ func performCastWithoutPrecisionTruncation( case types.TupleFamily: switch v := d.(type) { case *DTuple: + if t == types.AnyTuple { + // If AnyTuple is the target type, we can just use the input tuple. + return v, nil + } // To cast a Tuple to a Tuple, the lengths must be the same on both sides. // Then, each element is casted to the other element type. The labels of // the target type are kept. diff --git a/pkg/sql/sem/tree/type_check.go b/pkg/sql/sem/tree/type_check.go index ed9f3b302975..dde125d6fa41 100644 --- a/pkg/sql/sem/tree/type_check.go +++ b/pkg/sql/sem/tree/type_check.go @@ -462,7 +462,11 @@ func resolveCast( case toFamily == types.TupleFamily && fromFamily == types.TupleFamily: // Casts from tuple to tuple type succeed if the lengths of the tuples are // the same, and if there are casts resolvable across all of the elements - // pointwise. + // pointwise. Casts to AnyTuple are always allowed since they are + // implemented as a no-op. + if castTo == types.AnyTuple { + return nil + } fromTuple := castFrom.TupleContents() toTuple := castTo.TupleContents() if len(fromTuple) != len(toTuple) { @@ -2465,10 +2469,6 @@ func typeCheckTupleComparison( func typeCheckSameTypedTupleExprs( ctx context.Context, semaCtx *SemaContext, desired *types.T, exprs ...Expr, ) ([]TypedExpr, *types.T, error) { - // Hold the resolved type expressions of the provided exprs, in order. - // TODO(nvanbenschoten): Look into reducing allocations here. - typedExprs := make([]TypedExpr, len(exprs)) - // All other exprs must be tuples. first := exprs[0].(*Tuple) if err := checkAllExprsAreTuplesOrNulls(ctx, semaCtx, exprs[1:]); err != nil { @@ -2491,7 +2491,9 @@ func typeCheckSameTypedTupleExprs( sameTypeExprs = sameTypeExprs[:0] sameTypeExprsIndices = sameTypeExprsIndices[:0] for exprIdx, expr := range exprs { - if expr == DNull { + // Skip expressions that are not Tuple expressions (e.g. NULLs or CastExpr). + // They are checked at the end of this function. + if _, isTuple := expr.(*Tuple); !isTuple { continue } sameTypeExprs = append(sameTypeExprs, expr.(*Tuple).Exprs[elemIdx]) @@ -2511,11 +2513,24 @@ func typeCheckSameTypedTupleExprs( } resTypes.TupleContents()[elemIdx] = resType } + // Hold the resolved type expressions of the provided exprs, in order. + // TODO(nvanbenschoten): Look into reducing allocations here. + typedExprs := make([]TypedExpr, len(exprs)) for tupleIdx, expr := range exprs { - if expr != DNull { - expr.(*Tuple).typ = resTypes + if t, isTuple := expr.(*Tuple); isTuple { + // For Tuple exprs we can update the type with what we've inferred. + t.typ = resTypes + typedExprs[tupleIdx] = t + } else { + typedExpr, err := expr.TypeCheck(ctx, semaCtx, resTypes) + if err != nil { + return nil, nil, err + } + if !typedExpr.ResolvedType().EquivalentOrNull(resTypes, true /* allowNullTupleEquivalence */) { + return nil, nil, unexpectedTypeError(expr, resTypes, typedExpr.ResolvedType()) + } + typedExprs[tupleIdx] = typedExpr } - typedExprs[tupleIdx] = expr.(TypedExpr) } return typedExprs, resTypes, nil } @@ -2527,25 +2542,29 @@ func checkAllExprsAreTuplesOrNulls(ctx context.Context, semaCtx *SemaContext, ex _, isTuple := expr.(*Tuple) isNull := expr == DNull if !(isTuple || isNull) { + // We avoid calling TypeCheck on Tuple exprs since that causes the + // types to be resolved, which we only want to do later in type-checking. typedExpr, err := expr.TypeCheck(ctx, semaCtx, types.Any) if err != nil { return err } - return unexpectedTypeError(expr, types.AnyTuple, typedExpr.ResolvedType()) + if typedExpr.ResolvedType().Family() != types.TupleFamily { + return unexpectedTypeError(expr, types.AnyTuple, typedExpr.ResolvedType()) + } } } return nil } // checkAllTuplesHaveLength checks that all tuples in exprs have the expected -// length. Note that all nulls are skipped in this check. +// length. We only need to check Tuple exprs, since other expressions like +// CastExpr are handled later in type-checking 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 + if t, isTuple := expr.(*Tuple); isTuple { + if err := checkTupleHasLength(t, expectedLen); err != nil { + return err + } } } return nil diff --git a/pkg/sql/sem/tree/type_check_internal_test.go b/pkg/sql/sem/tree/type_check_internal_test.go index c9fe68294c42..4776378a9f75 100644 --- a/pkg/sql/sem/tree/type_check_internal_test.go +++ b/pkg/sql/sem/tree/type_check_internal_test.go @@ -260,7 +260,7 @@ func TestTypeCheckSameTypedExprs(t *testing.T) { {ptypesNone, types.Decimal, exprs(intConst("1"), placeholder(0)), types.Decimal, ptypesDecimal}, {ptypesNone, types.Decimal, exprs(decConst("1.1"), placeholder(0)), types.Decimal, ptypesDecimal}, } { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + t.Run(fmt.Sprintf("test_%d", i), func(t *testing.T) { attemptTypeCheckSameTypedExprs(t, i, d) }) } @@ -298,7 +298,9 @@ func TestTypeCheckSameTypedTupleExprs(t *testing.T) { // Verify desired type when possible with unresolved constants. {ptypesNone, ttuple(types.Int, types.Decimal), exprs(tuple(placeholder(0), intConst("1")), tuple(intConst("1"), placeholder(1))), ttuple(types.Int, types.Decimal), ptypesIntAndDecimal}, } { - attemptTypeCheckSameTypedExprs(t, i, d) + t.Run(fmt.Sprintf("test_%d", i), func(t *testing.T) { + attemptTypeCheckSameTypedExprs(t, i, d) + }) } } @@ -331,19 +333,20 @@ func TestTypeCheckSameTypedExprsError(t *testing.T) { } ctx := context.Background() for i, d := range testData { - semaCtx := tree.MakeSemaContext() - if err := semaCtx.Placeholders.Init(len(d.ptypes), d.ptypes); err != nil { - t.Error(err) - continue - } - desired := types.Any - if d.desired != nil { - desired = d.desired - } - forEachPerm(d.exprs, 0, func(exprs []copyableExpr) { - if _, _, err := tree.TypeCheckSameTypedExprs(ctx, &semaCtx, desired, buildExprs(exprs)...); !testutils.IsError(err, d.expectedErr) { - t.Errorf("%d: expected %s, but found %v", i, d.expectedErr, err) + t.Run(fmt.Sprintf("test_%d", i), func(t *testing.T) { + semaCtx := tree.MakeSemaContext() + if err := semaCtx.Placeholders.Init(len(d.ptypes), d.ptypes); err != nil { + t.Error(err) + } + desired := types.Any + if d.desired != nil { + desired = d.desired } + forEachPerm(d.exprs, 0, func(exprs []copyableExpr) { + if _, _, err := tree.TypeCheckSameTypedExprs(ctx, &semaCtx, desired, buildExprs(exprs)...); !testutils.IsError(err, d.expectedErr) { + t.Errorf("%d: expected %s, but found %v", i, d.expectedErr, err) + } + }) }) } } @@ -520,7 +523,7 @@ func TestProcessPlaceholderAnnotations(t *testing.T) { }, } for i, d := range testData { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + t.Run(fmt.Sprintf("test_%d", i), func(t *testing.T) { args := d.initArgs stmt := &tree.ValuesClause{Rows: []tree.Exprs{d.stmtExprs}} if err := tree.ProcessPlaceholderAnnotations(&semaCtx, stmt, args); err != nil {