Re: Changed SRF in targetlist handling

On 2016-08-03 20:22:03 -0700, Andres Freund wrote:
> On 2016-08-02 16:30:55 -0700, Andres Freund wrote:
> > > > Besides that I'm structurally wondering whether turning the original
> > > > query into a subquery is the right thing to do. It requires some kind of
> > > > ugly munching of Query->*, and has the above problem.
> > >
> > > It does not seem like it should be that hard, certainly no worse than
> > > subquery pullup. Want to show code?
> >
> > It's not super hard, there's some stuff like pushing/not-pushing
> > various sortgrouprefs to the subquery. But I think we can live with it.
> >
> > Let me clean up the code some, hope to have something today or
> > tomorrow.
>
> Here we go. This *clearly* is a POC, not more. But it mostly works.
>
>
> 0001 - adds some test, some of those change after the later patches
> 0002 - main SRF via ROWS FROM () implementation
> 0003 - Large patch removing now unused code. Most satisfying.
>
>
> The interesting bit is obviously 0002. What it basically does is, at the beginning
> of subquery_planner():
> 1) unsrfify:
> move the jointree into a subquery
> 2) unsrfify_reference_subquery_mutator:
> process the old targetlist to reference the new subquery. If a
> TargetEntry doesn't contain a set, it's entirely moved into the
> subquery. Otherwise all Vars/Aggrefs/... it references are moved to
> the subquery, and referenced in the outer query's target list.
> 3) unsrfify_implement_srfs_mutator:
> Replace set returning functions in the targetlist with references to
> a new FUNCTION RTE. All non-nested tSRFs are part of the same RTE
> (i.e. the least common multiple behaviour is gone). all tSRFs in
> arguments are implemented as another FUNCTION RTE.
>
> I discovered that we allow SRFs in UPDATE target lists. It's not clear
> to me what that's supposed to mean. Nor how exactly to implement that,
> given expand_targetlist(). Right now that fails with the patch, because
> it re-inserts Var's for the relation replaced by the subquery.
>
> Note that I've not bothered to fix up the regression test output - I'm
> certain that explain output and such will still change.
>
> Biggest questions / tasks:
> * General approach
> * DML handling
> * Operator implementation
> * SETOF record handling
> * correct handling of lateral dependency from RTE to subquery to force
> evaluation order, instead of my RangeTblEntry->deps hack.
> * lot of cleanup
>
> Comments?

Tom, do you think this is roughly going in the right direction? My plan
here is to develop two patches, to come before this:

a) Allow to avoid using a tuplestore for SRF_PERCALL SRFs in ROWS FROM -
otherwise our performance would regress noticeably in some cases.
b) Allow ROWS FROM() to return SETOF RECORD type SRFs as one column,
instead of expanded. That's important to be able move SETOF RECORD
returning functions in the targetlist into ROWS FROM, which otherwise
requires an explicit column list.

Greetings,

Andres Freund