Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (wasChanged SRF in targetlist handling) - Mailing list pgsql-hackers

From Andres Freund
Subject Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (wasChanged SRF in targetlist handling)
Date
Msg-id 20170116083316.2izqrxbvmyalazac@alap3.anarazel.de
Whole thread Raw
In response to Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (wasChanged SRF in targetlist handling)  (Andres Freund <andres@anarazel.de>)
Responses Re: [HACKERS] Implement targetlist SRFs using ROWS FROM() (wasChanged SRF in targetlist handling)  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
On 2017-01-15 19:29:52 -0800, Andres Freund wrote:
> On 2016-10-31 09:06:39 -0700, Andres Freund wrote:
> > On 2016-09-14 19:28:25 -0400, Tom Lane wrote:
> > > Andres Freund <andres@anarazel.de> writes:
> > > > On 2016-09-12 19:35:22 -0400, Tom Lane wrote:
> > > Here's a draft patch that is just meant to investigate what the planner
> > > changes might look like if we do it in the pipelined-result way.
> > > Accordingly, I didn't touch the executor, but just had it emit regular
> > > Result nodes for SRF-execution steps.  However, the SRFs are all
> > > guaranteed to appear at top level of their respective tlists, so that
> > > those Results could be replaced with something that works like
> > > nodeFunctionscan.
> >
> > > So I think we should continue investigating this way of doing things.
> > > I'll try to take a look at the executor end of it tomorrow.  However
> > > I'm leaving Friday for a week's vacation, and may not have anything to
> > > show before that.
> >
> > Are you planning to work on the execution side of things? I otherwise
> > can take a stab...
>
> Doing so now.

That worked quite well.  So we have a few questions, before I clean this
up:

- For now the node is named 'Srf' both internally and in explain - not sure if we want to make that something
longer/easierto understand for others? Proposals? TargetFunctionScan? SetResult?
 

- We could alternatively add all this into the Result node - it's not actually a lot of new code, and most of that is
boilerplatestuff about adding a new node.  I'm ok with both.
 

- I continued with the division of Labor that Tom had set up, so we're creating one Srf node for each "nested" set of
SRFs. We'd discussed nearby to change that for one node/path for all nested SRFs, partially because of costing.  But I
don'tlike the idea that much anymore. The implementation seems cleaner (and probably faster) this way, and I don't
thinknested targetlist SRFs are something worth optimizing for.  Anybody wants to argue differently?
 

- I chose to error out if a retset function appears in ExecEvalFunc/Oper and make both unconditionally set evalfunc to
ExecMakeFunctionResultNoSets. ExecMakeFunctionResult() is now externally visible.  That seems like the least noisy way
tochange things over, but I'm open for different proposals.
 

Comments?

Regards,

Andres



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: [HACKERS] check_srf_call_placement() isn't always setting p_hasTargetSRFs
Next
From: Masahiko Sawada
Date:
Subject: Re: [HACKERS] Transactions involving multiple postgres foreign servers