mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Allow functions that return sets of tuples to return simple NULLs.

Browse Source 
ExecMakeTableFunctionResult(), which is used in SELECT FROM function(...)
cases, formerly treated a simple NULL output from a function that both
returnsSet and returnsTuple as a violation of the SRF protocol.  What seems
better is to treat a NULL output as equivalent to ROW(NULL,NULL,...).
Without this, cases such as SELECT FROM unnest(...) on an array of
composite are vulnerable to unexpected and not-very-helpful failures.
Old code comments here suggested an alternative of just ignoring
simple-NULL outputs, but that doesn't seem very principled.

This change had been hung up for a long time due to uncertainty about
how much we wanted to buy into the equivalence of simple NULL and
ROW(NULL,NULL,...).  I think that's been mostly resolved by the discussion
around bug #14235, so let's go ahead and do it.

Per bug #7808 from Joe Van Dyk.  Although this is a pretty old report,
fixing it smells a bit more like a new feature than a bug fix, and the
lack of other similar complaints suggests that we shouldn't take much risk
of destabilization by back-patching.  (Maybe that could be revisited once
this patch has withstood some field usage.)

Andrew Gierth and Tom Lane

Report: <E1TurJE-0006Es-TK@wrigleys.postgresql.org>
This commit is contained in:
Tom Lane 2016-07-26 21:33:49 -04:00
parent 976b24fb47
commit d8411a6c8b
3 changed files with 106 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

125
src/backend/executor/execQual.c
View File

@ -2229,45 +2229,16 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
break;
/*
* Can't do anything very useful with NULL rowtype values. For a
* function returning set, we consider this a protocol violation
* (but another alternative would be to just ignore the result and
* "continue" to get another row). For a function not returning
* set, we fall out of the loop; we'll cons up an all-nulls result
* row below.
*/
if (returnsTuple && fcinfo.isnull)
{
if (!returnsSet)
break;
ereport(ERROR,
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
errmsg("function returning set of rows cannot return null value")));
}
/*
* If first time through, build tupdesc and tuplestore for result
* If first time through, build tuplestore for result. For a
* scalar function result type, also make a suitable tupdesc.
*/
if (first_time)
{
oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
if (returnsTuple)
tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
rsinfo.setResult = tupstore;
if (!returnsTuple)
{
/*
* Use the type info embedded in the rowtype Datum to look
* up the needed tupdesc. Make a copy for the query.
*/
HeapTupleHeader td;
td = DatumGetHeapTupleHeader(result);
tupdesc = lookup_rowtype_tupdesc_copy(HeapTupleHeaderGetTypeId(td),
HeapTupleHeaderGetTypMod(td));
}
else
{
/*
* Scalar type, so make a single-column descriptor
*/
tupdesc = CreateTemplateTupleDesc(1, false);
TupleDescInitEntry(tupdesc,
(AttrNumber) 1,
@ -2275,11 +2246,9 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
funcrettype,
-1,
0);
rsinfo.setDesc = tupdesc;
}
tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
MemoryContextSwitchTo(oldcontext);
rsinfo.setResult = tupstore;
rsinfo.setDesc = tupdesc;
}
/*
@ -2287,31 +2256,69 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
*/
if (returnsTuple)
{
HeapTupleHeader td;
if (!fcinfo.isnull)
{
HeapTupleHeader td = DatumGetHeapTupleHeader(result);
td = DatumGetHeapTupleHeader(result);
if (tupdesc == NULL)
{
/*
* This is the first non-NULL result from the
* function. Use the type info embedded in the
* rowtype Datum to look up the needed tupdesc. Make
* a copy for the query.
*/
oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
tupdesc = lookup_rowtype_tupdesc_copy(HeapTupleHeaderGetTypeId(td),
HeapTupleHeaderGetTypMod(td));
rsinfo.setDesc = tupdesc;
MemoryContextSwitchTo(oldcontext);
}
else
{
/*
* Verify all later returned rows have same subtype;
* necessary in case the type is RECORD.
*/
if (HeapTupleHeaderGetTypeId(td) != tupdesc->tdtypeid ||
HeapTupleHeaderGetTypMod(td) != tupdesc->tdtypmod)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("rows returned by function are not all of the same row type")));
}
/*
* Verify all returned rows have same subtype; necessary in
* case the type is RECORD.
*/
if (HeapTupleHeaderGetTypeId(td) != tupdesc->tdtypeid ||
HeapTupleHeaderGetTypMod(td) != tupdesc->tdtypmod)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("rows returned by function are not all of the same row type")));
/*
* tuplestore_puttuple needs a HeapTuple not a bare
* HeapTupleHeader, but it doesn't need all the fields.
*/
tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
tmptup.t_data = td;
/*
* tuplestore_puttuple needs a HeapTuple not a bare
* HeapTupleHeader, but it doesn't need all the fields.
*/
tmptup.t_len = HeapTupleHeaderGetDatumLength(td);
tmptup.t_data = td;
tuplestore_puttuple(tupstore, &tmptup);
}
else
{
/*
* NULL result from a tuple-returning function; expand it
* to a row of all nulls. We rely on the expectedDesc to
* form such rows. (Note: this would be problematic if
* tuplestore_putvalues saved the tdtypeid/tdtypmod from
* the provided descriptor, since that might not match
* what we get from the function itself. But it doesn't.)
*/
int natts = expectedDesc->natts;
bool *nullflags;
tuplestore_puttuple(tupstore, &tmptup);
nullflags = (bool *) palloc(natts * sizeof(bool));
memset(nullflags, true, natts * sizeof(bool));
tuplestore_putvalues(tupstore, expectedDesc, NULL, nullflags);
}
}
else
{
/* Scalar-type case: just store the function result */
tuplestore_putvalues(tupstore, tupdesc, &result, &fcinfo.isnull);
}
/*
* Are we done?
@ -2343,7 +2350,8 @@ no_function_result:
/*
* If we got nothing from the function (ie, an empty-set or NULL result),
* we have to create the tuplestore to return, and if it's a
* non-set-returning function then insert a single all-nulls row.
* non-set-returning function then insert a single all-nulls row. As
* above, we depend on the expectedDesc to manufacture the dummy row.
*/
if (rsinfo.setResult == NULL)
{
@ -2353,15 +2361,12 @@ no_function_result:
if (!returnsSet)
{
int natts = expectedDesc->natts;
Datum *nulldatums;
bool *nullflags;
MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
nulldatums = (Datum *) palloc0(natts * sizeof(Datum));
nullflags = (bool *) palloc(natts * sizeof(bool));
memset(nullflags, true, natts * sizeof(bool));
MemoryContextSwitchTo(econtext->ecxt_per_query_memory);
tuplestore_putvalues(tupstore, expectedDesc, nulldatums, nullflags);
tuplestore_putvalues(tupstore, expectedDesc, NULL, nullflags);
}
}

30
src/test/regress/expected/rangefuncs.out
View File

@ -2076,3 +2076,33 @@ select x from int8_tbl, extractq2_2_opt(int8_tbl) f(x);
-4567890123456789
(5 rows)
-- check handling of nulls in SRF results (bug #7808)
create type foo2 as (a integer, b text);
select *, row_to_json(u) from unnest(array[(1,'foo')::foo2, null::foo2]) u;
a | b | row_to_json
---+-----+---------------------
1 | foo | {"a":1,"b":"foo"}
| | {"a":null,"b":null}
(2 rows)
select *, row_to_json(u) from unnest(array[null::foo2, null::foo2]) u;
a | b | row_to_json
---+---+---------------------
| | {"a":null,"b":null}
| | {"a":null,"b":null}
(2 rows)
select *, row_to_json(u) from unnest(array[null::foo2, (1,'foo')::foo2, null::foo2]) u;
a | b | row_to_json
---+-----+---------------------
| | {"a":null,"b":null}
1 | foo | {"a":1,"b":"foo"}
| | {"a":null,"b":null}
(3 rows)
select *, row_to_json(u) from unnest(array[]::foo2[]) u;
a | b | row_to_json
---+---+-------------
(0 rows)
drop type foo2;

11
src/test/regress/sql/rangefuncs.sql
View File

@ -639,3 +639,14 @@ explain (verbose, costs off)
select x from int8_tbl, extractq2_2_opt(int8_tbl) f(x);
select x from int8_tbl, extractq2_2_opt(int8_tbl) f(x);
-- check handling of nulls in SRF results (bug #7808)
create type foo2 as (a integer, b text);
select *, row_to_json(u) from unnest(array[(1,'foo')::foo2, null::foo2]) u;
select *, row_to_json(u) from unnest(array[null::foo2, null::foo2]) u;
select *, row_to_json(u) from unnest(array[null::foo2, (1,'foo')::foo2, null::foo2]) u;
select *, row_to_json(u) from unnest(array[]::foo2[]) u;
drop type foo2;