On Sat, Dec 16, 2017 at 9:13 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Fri, Dec 15, 2017 at 2:10 PM, Jeff Janes <jeff.ja...@gmail.com> wrote: >> I had hit on the same change. And was also surprised that it was located >> where it was. With the change, it uses the parallel plan all the way down >> to LIMIT 1. >> >> With the patch, it still satisfies make check, so if it introduces errors >> they are subtle ones. If we can't actually do this and it needs to stay -1, >> then I think we need a comment to explain why. > > Interesting. I suspect this is correct now, but would not have been > before commit 3452dc5240da43e833118484e1e9b4894d04431c. AFAICS, this > doesn't affect any execution-time behavior, just the cost estimate. > And, prior to that commit, the execution-time behavior was different: > there would not have been any way for the worker to do a top-N sort, > because the LIMIT was not pushed through the Gather. > > Does that sound right, or am I still confused?
Looks right to me. Commit 3452dc52 just forgot to tell the planner. I'm pleased about that because it makes this a slam-dunk bug-fix and not some confusing hard to justify costing problem. BTW with this patch, the same test using a smaller foo table with 250k rows limit 1 runs a parallel plan in 45ms here, but 220k rows limit 1 runs a non-parallel plan in 80ms, but that's just a regular costing problem where the point at which the curves cross over will be hard to get right due to all kinds of other variables, so I guess that kind of thing is expected. PS Oops, I do actually know how to spell "versus". Typos are so much more embarrassing in subject lines! -- Thomas Munro http://www.enterprisedb.com
