Add tests for PL/pgSQL SRFs
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t50205psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 20, 2026 at 04:15 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t50205_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t50205_1 && git checkout t50205_1Patchset v1 (message #1) is on t50205_1
Hello Hackers,
While working on inlining non-SQL SRFs [1]https://commitfest.postgresql.org/49/5083/ I noticed we don't have tests for when a PL/pgSQL
function requires materialize mode but doesn't have a result TupleDesc. Here is a patch adding tests
for that, as well as some other conditions around SRF calls with `SETOF RECORD` vs `TABLE (...)`.
There aren't any code changes, just some new tests.
But IMO it might be better to change the code. This error message is a bit confusing:
+-- materialize mode requires a result TupleDesc:
+select array_to_set2(array['one', 'two']); -- fail
+ERROR: materialize mode required, but it is not allowed in this context
+CONTEXT: PL/pgSQL function array_to_set2(anyarray) line 3 at RETURN QUERYPerhaps it would be better to give the same error as here?:
+select * from array_to_set2(array['one', 'two']); -- fail
+ERROR: a column definition list is required for functions returning "record"
+LINE 1: select * from array_to_set2(array['one', 'two']);If folks agree, I can work on a patch for that. Otherwise, at least this patch documents the current
behavior and increases coverage.
[1]: https://commitfest.postgresql.org/49/5083/
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com
Paul Jungwirth <pj@illuminatedcomputing.com> writes:
While working on inlining non-SQL SRFs [1] I noticed we don't have tests for when a PL/pgSQL
function requires materialize mode but doesn't have a result TupleDesc. Here is a patch adding tests
for that, as well as some other conditions around SRF calls with `SETOF RECORD` vs `TABLE (...)`.
There aren't any code changes, just some new tests.
AFAICT this test case adds coverage of exactly one line of code,
and that line is an unremarkable ereport(ERROR). I can't get
excited about spending test cycles on this forevermore, especially
not core-regression-test cycles.
But IMO it might be better to change the code. This error message is a bit confusing:
+-- materialize mode requires a result TupleDesc: +select array_to_set2(array['one', 'two']); -- fail +ERROR: materialize mode required, but it is not allowed in this context +CONTEXT: PL/pgSQL function array_to_set2(anyarray) line 3 at RETURN QUERY
A quick grep shows that there are ten other places throwing exactly
this same error message. About half of them are throwing it strictly
for
if (!(rsinfo->allowedModes & SFRM_Materialize))
and I think that that's a reasonable way to report that condition.
But the other half are throwing in other conditions such as
if (!(rsinfo->allowedModes & SFRM_Materialize) ||
rsinfo->expectedDesc == NULL)
and I agree with you that maybe we're being too lazy there.
I could get behind separate error messages for these conditions,
like
if (!(rsinfo->allowedModes & SFRM_Materialize))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("materialize mode required, but it is not allowed in this context")));
if (rsinfo->expectedDesc == NULL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("a column definition list is required for functions returning \"record\"")));
It's not quite clear to me if that's the same thing you're suggesting?
I'm also a bit uncomfortable with using that phrasing of the second
error, because it seems to be making assumptions that are well beyond
what this code knows to be true. Is it possible to get here in a
function that *doesn't* return record? Maybe we should just say
"a column definition list is required for this function", or words
to that effect (throwing in the function name might be good).
In any case, if we do something about it I'd want to do so in all
of the places that are taking similar shortcuts, not only plpgsql.
A different line of thought is to try to remove this implementation
restriction, but I've not looked at what that would entail.
regards, tom lane