Re: Changed SRF in targetlist handling

On 2016-05-23 13:10:29 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > One idea I circulated was to fix that by interjecting a special executor
> > node to process SRF containing targetlists (reusing Result possibly?).
> > That'd allow to remove the isDone argument from ExecEval*/ExecProject*
> > and get rid of ps_TupFromTlist which is fairly ugly.
>
> Would that not lead to, in effect, duplicating all of execQual.c? The new
> executor node would still have to be prepared to process all expression
> node types.

I don't think it necessarily has to. ISTM that if we add a version of
ExecProject()/ExecTargetList() that continues returning multiple rows,
we can make the knowledge about the one type of expression we allow to
return multiple rows. That'd require a bit of uglyness to implement
stuff like
SELECT generate_series(1, 2)::text, generate_series(1, 2) * 5;
etc. It seems we'd basically have to do one projection step for the
SRFs, and then another for the rest. I'm inclined to think that's
acceptable to get rid of a lot of the related uglyness.

> > One issue with removing targetlist SRFs is that they're currently
> > considerably faster than SRFs in FROM:
>
> I suspect that depends greatly on your test case. But in any case
> we could put more effort into optimizing nodeFunctionscan.

I doubt you'll find cases where it's significantly the other way round
for percall SRFs. The fundamental issue is that targetlist SRFs don't
have to spill to a tuplestore, whereas nodeFunctionscan ones have to
(even if they're percall).