Re: Query fails when SRFs are part of FROM clause (Commit id: 69f4b9c85f)

On 2017-01-30 18:54:50 -0500, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > Wonder if we there's an argument to be made for implementing this
> > roughly similarly to split_pathtarget_at_srf - instead of injecting a
> > ProjectSet node we'd add a FunctionScan node below a Result node.
>
> Yeah, possibly. That would have the advantage of avoiding an ExecProject
> step when the SRFs aren't buried, which would certainly be the expected
> case.
>
> If you don't want to make ExecInitExpr responsible, then the planner would
> have to do something like split_pathtarget_at_srf anyway to decompose the
> expressions, no matter which executor representation we use.

Working on this I came across a few things:

Splitting things away from the FunctionScan node doesn't work entirely
naturally, due to ORDINALITY. Consider e.g. cases where there's no
function to evaluate anymore, because it got inlined, but we want to
ORDINALITY. We could obviously support FunctionScan nodes without any
associated (or empty) RangeTblFunctions, but that seems awkward.

Secondly, doing non-function stuff gets interesting when volatility is
involved: Consider something like FROM CAST(srf() * volatile_func() AS
whatnot); when, and how often, does volatile_func() get evaluated? If
we put it somewhere around the projection path it'll only get evaluated
if the rows are actually retrieved and will get re-evaluated upon
projection. If we put it somewhere below the tuplestores that
nodeFunctionscan./execSRF.c generate for each RangeTblFunction, it'll
get called evaluated regardless of being retrieved. The latter is
noticeably more complicated, because of SRFM_Materialize SRFs. Our
policy around when it's ok to re-evaluate volatile functions isn't clear
enough to me, to say which one is right and which one is wrong.

For now there's simply another RangeTblFunctions field with a separate
non-FuncExpr expression, that can reference to the SRF output via scan
Vars. That's evaluated in FunctionNext's !simple branch, where we
conveniently have a separate slot already. The separation currently
happens create_functionscan_plan().

Tom, do you have any opinion on the volatility stuff?

- Andres