Re: [HACKERS] [PATCH] Push limit to sort through a subquery - Mailing list pgsql-hackers
| From | Douglas Doole |
|---|---|
| Subject | Re: [HACKERS] [PATCH] Push limit to sort through a subquery |
| Date | |
| Msg-id | CADE5jYK4Cm-vJ3bG2J1aNvFucq83NbRfQXe1h5W0pEWY+hXJCg@mail.gmail.com Whole thread Raw |
| In response to | Re: [HACKERS] [PATCH] Push limit to sort through a subquery (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>) |
| Responses |
Re: [HACKERS] [PATCH] Push limit to sort through a subquery
(Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
|
| List | pgsql-hackers |
Thanks for the feedback Ashutosh.
I disagree that we need to call pass_down_bound() recursively. The whole point of the recursive call would be to tunnel through a plan node. Since SubqueryScanState only has one child (unlike MergeAppendState) this can be more efficiently done inline rather than via a recursive call.
In the case of ResultState, this is classic tail-end recursion and the C compiler should optimize it out. As we add more recursive calls though, it becomes harder for the compiler to do the right thing. (I'll admit that I'm not up on what state-of-the-art in recursion removal though, so maybe my concern is moot. That said, I prefer not to depend on compiler optimizations if there's a clean alternative way to express the code.)
I do agree that it would make the code cleaner to handle ResultState and SubqueryScanState in a similar fashion. My preference, however, would be to remove the recursive call from ResultState handling. That would give us code like:
if child == SubqueryScanState
child = SubqueryScanState->child
else if child == ResultState
child = ResultState->child
if child == SortState
limit pushdown
else if child == MergeAppendState
recursive call on all children
If it is possible to get more than one SubqueryScanState and/or ResultState between the limit and sort, then the first block of code could be placed in a while loop.
If you'd prefer that I rework the patch as I discussed, just let me know and I'll resubmit it.
- Doug
Salesforce
On Wed, Apr 19, 2017 at 1:57 AM Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:
The function pass_down_bound() is a recursive function. For
SubqueryScanState we have to do something similar to ResultState i.e.
call pass_down_bound() recursively on subqueryScanState->subplan.
Please add this to the next commitfest.
On Wed, Apr 19, 2017 at 6:09 AM, Douglas Doole <dougdoole@gmail.com> wrote:
> We've hit a case where pass_down_bound() isn't pushing the row count limit
> from limit into sort. The issue is that we're getting a subquery scan node
> between the limit and the sort. The subquery is only doing column projection
> and has no quals or SRFs so it should be safe to push the limit into the
> sort.
>
> The query that hit the problem can be simplified to:
>
> SELECT * FROM (SELECT A,B FROM T ORDER BY C) LIMIT 5
>
> (Yeah, the query's a little screwy in that the ORDER BY should really be
> outside the subselect, but it came from a query generator, so that's a
> different conversation.)
>
> Proposed patch is attached.
>
> - Doug
> Salesforce
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
pgsql-hackers by date: