Re: Changed SRF in targetlist handling

On Mon, May 23, 2016 at 12:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.

+1 on removing LCM.

The behavior of multiple targetlist SRF is so bizarre that it's
incredible to believe anyone would reasonably expect it to work that
way. Agree also that casual sane usage of target list SRF is
incredibly common via generate_series() and unnest() etc is
exceptionally common...better not to break those cases without a
better justification than code simplicity.

merlin