On 2017-01-30 18:54:50 -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
