Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)

Hi,

On 2016-09-14 19:28:25 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2016-09-12 19:35:22 -0400, Tom Lane wrote:
> >> Anyway I'll draft a prototype and then we can compare.
>
> > Ok, cool.
>
> 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.

Nice.

> A difficulty with this restriction is that if you have a query like
> "select f1, generate_series(1,2) / 10 from tab" then you end up with both
> a SRF-executing Result and a separate scalar-projection Result above it,
> because the division-by-ten has to happen in a separate plan level.

Makes sense. I guess we could teach the SRF pipeline node to execute a
series of such steps. Hm. That makes me think of something:

Hm. One thing I wonder about with this approach, is how we're going to
handle something absurd like:
SELECT generate_series(1, generate_series(1, 2)), generate_series(1, generate_series(2,4));

I guess we have to do here is
Step: generate_series(1,2), 1, 2, 4
Step: generate_series(1, Var(generate_series(1,2))), 1, 2, 4
Step: Var(generate_series(1, Var(generate_series(1,2)))), 1, generate_series(2, 4)
Step: Var(generate_series(1, Var(generate_series(1,2)))), generate_series(1, Var(generate_series(2, 4)))

But that'd still not have the same lockstepping behaviour, right? I'm
at a conference, and half-ill, so I might just standing on my own brain
here.

> The planner's notions about the cost of Result make it think that this is
> quite expensive --- mainly because the upper Result will be iterated once
> per SRF output row, so that you get hit with cpu_tuple_cost per output row.
> And that in turn leads it to change plans in one or two cases in the
> regression tests. Maybe that's fine. I'm worried though that it's right
> that this will be unduly expensive. So I'm kind of tempted to define the
> SRF-executing node as acting more like, say, Agg or WindowFunc, in that
> it has a list of SRFs to execute and then it has the ability to project a
> scalar tlist on top of those results.

Hah, was thinking the same above ;)

> On the whole I'm pretty pleased with this approach, at least from the
> point of view of the planner. The net addition of planner code is
> smaller than what you had,

Not by much. But I do agree that there's some advantages here.

> and though I'm no doubt biased, I think this
> version is much cleaner.

Certainly seems a bit easier to extend and adjust behaviour. Not having
to deal with enforcing join order, and having less issues with
determining what to push where is certainly advantageous. After all,
that was why I initially was thinking of tis approach.

> Also, though this patch doesn't address exactly
> how we might do it, it's fairly clear that it'd be possible to allow
> FDWs and CustomScans to implement SRF execution, eg pushing a SRF down to
> a foreign server, in a reasonably straightforward extension of the
> existing upper-pathification hooks. If we go with the lateral function
> RTE approach, that's going to be somewhere between hard and impossible.

Hm. Not sure if there's that much point in doing that, but I agree that
the LATERAL approach adds more restrictions.

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

If you have something that's halfway recognizable, could you perhaps
post it?

Regards,

Andres