Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

On 2016-09-02 14:04:24 +0530, Robert Haas wrote:
> On Sun, Aug 28, 2016 at 3:18 AM, Andres Freund <andres@anarazel.de> wrote:
> > 0003-Avoid-materializing-SRFs-in-the-FROM-list.patch
> >   To avoid performance regressions from moving SRFM_ValuePerCall SRFs to
> >   ROWS FROM, nodeFunctionscan.c needs to support not materializing
> >   output.
> >
> >   In my present patch I've *ripped out* the support for materialization
> >   in nodeFunctionscan.c entirely. That means that rescans referencing
> >   volatile functions can change their behaviour (if a function is
> >   rescanned, without having it's parameters changed), and that native
> >   backward scan support is gone.  I don't think that's actually an issue.
> 
> Can you expand on why you think those things aren't an issue?  Because
> it seems like they might be.

Backward scans are, by the planner, easily implemented by adding a
materialize node. Which will, when ordinality or multiple ROWS FROM
expressions are present, even be more runtime & memory efficient.  I
also don't think all that many people use FOR SCROLL cursors over SRFs
containing queries.

The part about rewinding is a bit more complicated. As of HEAD, a
rewound scan where some of the SRFs have to change due to parameter
inputs, but others don't, will only re-compute the ones with parameter
changes.  I don't think it's more confusing to rescan the entire input,
rather parts of it in that case.  If the entire input is re-scanned, the
planner knows how to materialize the entire scan output.

I think it'd be pretty annoying to continue to always materialize
ValuePerCall SRFs just to support that type of re-scan behaviour. We
don't really, to my knowledge, flag well whether rescans are required
atm, so we can't even easily do it conditionally.

Greetings,

Andres Freund