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

On 2016-09-15 15:23:58 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > 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.
>
> I think you are wrong on this not being an issue: it is critical that
> rescan deliver the same results as before, else for example having a
> function RTE on the inside of a nestloop will give nonsensical/broken
> results.

I find that quite unconvincing. We quite freely re-evaluate functions in
the targetlist again, even if they're volatile and/or SRFs.

If we implement tSRFs as pipeline nodes, we can "simply" default to the
never materializing behaviour there I guess.

> Moreover, I think we'd all agreed that this effort needs to avoid any
> not-absolutely-necessary semantics changes.

I don't agree with that. Adding pointless complications for a niche
edge cases of niche features isn't worth it.

> Another idea is that we could extend the set of ExecInitNode flags
> (EXEC_FLAG_REWIND etc) to tell child nodes whether they need to implement
> rescan correctly in this sense; if they are not RHS children of nestloops,
> and maybe one or two other cases, they don't. That would give another
> route by which nodeFunctionscan could decide that it can skip
> materialization in common cases.

That's something I've wondered about too. Materializing if rescans are
required is quite acceptable, and probably rather rare in
practice. Seems not unlikely that that information would be valuable for
other node types too.

Regards,

Andres