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(at)anarazel(dot)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