Re: [PATCH] Push limit to sort through a subquery

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
>