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.ba...@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 <dougdo...@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 (email@example.com) > > To make changes to your subscription: > > http://www.postgresql.org/mailpref/pgsql-hackers > > > > > > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company >