Skip to content

Commit

Permalink
Merge #110956
Browse files Browse the repository at this point in the history
110956: sql: return expected user-facing error for invalid unnest arguments r=DrewKimball a=DrewKimball

Previously, the `unnest` builtin function could trigger an internal error when passed multiple non array-type arguments. This is because it only checked whether the arguments were `NULL` when determining whether they were of a valid type. This is not a problem for some types, like `INT`, because they will prevent the function overloads from being resolved.

However, since `TEXT` arguments can be cast to `ARRAY` types, function overload resolution succeeds. Since `unnest` only checked for `NULL`, it would assume that the arguments were array types, and attempt to retrieve the (nil) array contents. For the single-argument case this wasn't a problem because nil is used to signal invalid arguments, anyway. However, the multiple-argument case wraps the array contents of each argument type into a tuple, resulting in a tuple-type of nil types. This caused a nil-pointer dereference later down the line.

This patch prevents the internal error by checking directly that the arguments are `ARRAY` types, to ensure that the array contents are non-nil. If the check fails, the (nil) `tree.UnknownReturnType` type is returned, which signals an invalid type. That results in an expected, user-facing error instead of an internal error.

The `information_schema._pg_expandarray` builtin function had a similar vulnerability. This patch fixes that as well.

Fixes #110952

Release note (bug fix): Fixed an edge case in the `unnest` and `information_schema._pg_expandarray` builtin functions that could cause an internal error when passed string arguments that could be cast to an array.

Co-authored-by: Drew Kimball <[email protected]>
  • Loading branch information
craig[bot] and DrewKimball committed Sep 22, 2023
2 parents e9a8c49 + 1f24340 commit ed75b08
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
44 changes: 44 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/srfs
Original file line number Diff line number Diff line change
Expand Up @@ -1422,3 +1422,47 @@ NULL
NULL
4
5

# Regression test for #110952 - providing invalid arguments to unnest should
# not trigger an internal error.
statement error pgcode 42804 pq: could not determine polymorphic type: unnest\(string\)
SELECT unnest('{}');

statement error pgcode 42804 pq: could not determine polymorphic type: unnest\(string, string, string\)
SELECT unnest('{}', '{}', '{}');

statement error pgcode 42883 pq: unknown signature: unnest\(int\)
SELECT unnest(1);

statement error pgcode 42883 pq: unknown signature: unnest\(int, int, int\)
SELECT unnest(1, 2, 3);

statement error pgcode 42804 pq: could not determine polymorphic type: unnest\(unknown\)
SELECT unnest(NULL);

statement error pgcode 42804 pq: could not determine polymorphic type: unnest\(unknown, unknown, unknown\)
SELECT unnest(NULL, NULL, NULL);

# pg_expandarray handles return types similarly to unnest.
statement error pgcode 42804 pq: could not determine polymorphic type: information_schema._pg_expandarray\(string\)
SELECT information_schema._pg_expandarray('{}');

query T
SELECT unnest('{1}'::int[], '{2}', '{3}');
----
(1,2,3)

query T
SELECT unnest('{1}'::int[], '{}', '{}');
----
(1,,)

query T
SELECT unnest('{1}', '{2}', '{3}'::int[]);
----
(1,2,3)

query T
SELECT unnest('{}', '{}', '{3}'::int[]);
----
(,,3)
6 changes: 3 additions & 3 deletions pkg/sql/sem/builtins/generator_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ var generators = map[string]builtinDefinition{
makeGeneratorOverloadWithReturnType(
tree.ParamTypes{{Name: "input", Typ: types.AnyArray}},
func(args []tree.TypedExpr) *types.T {
if len(args) == 0 || args[0].ResolvedType().Family() == types.UnknownFamily {
if len(args) == 0 || args[0].ResolvedType().Family() != types.ArrayFamily {
return tree.UnknownReturnType
}
return args[0].ResolvedType().ArrayContents()
Expand All @@ -319,7 +319,7 @@ var generators = map[string]builtinDefinition{
returnTypes := make([]*types.T, len(args))
labels := make([]string, len(args))
for i, arg := range args {
if arg.ResolvedType().Family() == types.UnknownFamily {
if arg.ResolvedType().Family() != types.ArrayFamily {
return tree.UnknownReturnType
}
returnTypes[i] = arg.ResolvedType().ArrayContents()
Expand All @@ -337,7 +337,7 @@ var generators = map[string]builtinDefinition{
makeGeneratorOverloadWithReturnType(
tree.ParamTypes{{Name: "input", Typ: types.AnyArray}},
func(args []tree.TypedExpr) *types.T {
if len(args) == 0 || args[0].ResolvedType().Family() == types.UnknownFamily {
if len(args) == 0 || args[0].ResolvedType().Family() != types.ArrayFamily {
return tree.UnknownReturnType
}
t := args[0].ResolvedType().ArrayContents()
Expand Down

0 comments on commit ed75b08

Please sign in to comment.