On Fri, Aug 25, 2017 at 9:24 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Basically, this argument is that we should contort the code in order
> to avoid tail recursion, rather than assuming the compiler will turn
> that recursion into iteration, even in cases where there is really
> zero evidence that we should be worrying about performance at all
> rather than correctness and readability. I do not agree (and just
> finished pushing a patch to change it).
I'm not prepared to argue further about it. This strikes me as one of
those things that is really a matter of judgement; I don't really care
much about how the code ends up but I think your portrayal of what was
there is unduly negative.
>> On another note, here's a second patch applying on top of my earlier
>> patch to push down Limit through Gather and Gather Merge.
>
> Can you demonstrate any case where the planner would put Gather between
> Sort and Limit? The GatherMerge case is probably interesting, but I'm
> having doubts about Gather.
That's an interesting point. I was thinking that we needed both of
these patches in order to make the proposed regression test pass,
because in the relevant case we'll have Gather -> Limit -> Sort, but
actually we only need the second one, because as you point out here
what matters is what's between the Limit and the Sort. I was thinking
that we'd need to be able to see through the Gather at the top of the
plan, but we don't.
However, there's another opportunity for optimization here, which is
the Limit -> Gather -> Anything case. A worker which has itself
generated enough tuples to fill the limit can certainly stop working,
but right now it doesn't know that. The patch I posted before didn't
think about that case, but it seems to require only trivial
modifications. I was going to post an updated patch trying to
address that, but I see that you've already posted something, so I'll
go have a look at that instead.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company