Hi folks,
I’ve updated the patch, addressed the rescan issue, and restructured the tests.
I’ve taken a slightly different approach this time, re-using the (already pipeline-supporting) machinery of the Materialize node, and extended it to allow an SFRM_Materialize SRF to donate the tuplestore it returns. I feel this yields a better code structure, as well getting as more reuse.
It also opens up more informative and transparent EXPLAIN output. For example, the following shows Materialize explicitly, whereas previously a FunctionScan would have silently materialised the result of both generate_series() invocations.
postgres=# explain (analyze, costs off, timing off, summary off)
select * from generate_series(11,15) r, generate_series(11,14) s;
QUERY PLAN
------------------------------------------------------------------
Nested Loop (actual rows=20 loops=1)
-> Function Scan on generate_series s (actual rows=4 loops=1)
-> SRF Scan (actual rows=4 loops=1)
SFRM: ValuePerCall
-> Function Scan on generate_series r (actual rows=5 loops=4)
-> Materialize (actual rows=5 loops=4)
-> SRF Scan (actual rows=5 loops=1)
SFRM: ValuePerCall
I also thought again about when to materialise, and particularly Robert’s suggestion[1] (which is in also this thread, but I didn’t originally understand the implication of). If I’m not wrong, between occasional explicit use of a Materialize node by the planner, and more careful observation of EXEC_FLAG_REWIND and EXEC_FLAG_BACKWARD in FunctionScan’s initialisation, we do actually have what is needed to pipeline without materialisation in at least some cases. There is not a mechanism to preferentially re-execute a SRF rather than materialise it, but because materialisation only seems to be necessary in the face of a join or a scrollable cursor, I’m not considering much of a problem anymore.
The EXPLAIN output needs a bit of work, costing is still a sore point, and it’s not quite as straight-line performant as my first attempt, as well as there undoubtedly being some unanticipated breakages and rough edges.
But the concept seems to work roughly as I intended (i.e., allowing FunctionScan to pipeline). Unless there are any objections, I will push it into the January commit fest for progressing.
(Revised patch attached.)
denty.