Re: Changed SRF in targetlist handling

Andres Freund <andres@anarazel.de> writes:
> discussing executor performance with a number of people at pgcon,
> several hackers - me included - complained about the additional
> complexity, both code and runtime, required to handle SRFs in the target
> list.

Yeah, this has been an annoyance for a long time.

> 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.

> Robert suggested - IIRC mentioning previous on-list discussion - to
> instead rewrite targetlist SRFs into lateral joins. My gut feeling is
> that that'd be a larger undertaking, with significant semantics changes.

Yes, this was discussed on-list awhile back (I see David found a reference
already).  I think it's feasible, although we'd first have to agree
whether we want to remain bug-compatible with the old
least-common-multiple-of-the-periods behavior.  I would vote for not,
but it's certainly a debatable thing.

> If we accept bigger semantical changes, I'm inclined to instead just get
> rid of targetlist SRFs in total; they're really weird and not needed
> anymore.

This seems a bridge too far to me.  It's just way too common to do
"select generate_series(1,n)".  We could tell people they have to
rewrite to "select * from generate_series(1,n)", but it would be far
more polite to do that for them.

> 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.

			regards, tom lane