On Thu, Jul 4, 2019 at 10:46 AM Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On Thu, Jul 04, 2019 at 09:29:49AM -0400, James Coleman wrote: > >On Tue, Jun 25, 2019 at 7:22 PM Tomas Vondra > ><tomas.von...@2ndquadrant.com> wrote: > >> > >> On Tue, Jun 25, 2019 at 04:53:40PM -0400, James Coleman wrote: > >> > > >> >Unrelated: if you or someone else you know that's more familiar with > >> >the parallel code, I'd be interested in their looking at the patch at > >> >some point, because I have a suspicion it might not be operating in > >... > >> So I've looked into that, and the reason seems fairly simple - when > >> generating the Gather Merge paths, we only look at paths that are in > >> partial_pathlist. See generate_gather_paths(). > >> > >> And we only have sequential + index paths in partial_pathlist, not > >> incremental sort paths. > >> > >> IMHO we can do two things: > >> > >> 1) modify generate_gather_paths to also consider incremental sort for > >> each sorted path, similarly to what create_ordered_paths does > >> > >> 2) modify build_index_paths to also generate an incremental sort path > >> for each index path > >> > >> IMHO (1) is the right choice here, because it automatically does the > >> trick for all other types of ordered paths, not just index scans. So, > >> something like the attached patch, which gives me plans like this: > >... > >> But I'm not going to claim those are total fixes, it's the minimum I > >> needed to do to make this particular type of plan work. > > > >Thanks for looking into this! > > > >I intended to apply this to my most recent version of the patch (just > >sent a few minutes ago), but when I apply it I noticed that the > >partition_aggregate regression tests have several of these failures: > > > >ERROR: could not find pathkey item to sort > > > >I haven't had time to look into the cause yet, so I decided to wait > >until the next patch revision. > > > > FWIW I don't claim the patch I shared is complete and/or 100% correct. > It was more an illustration of the issue and the smallest patch to make > a particular query work. The test failures are a consequence of that. > > I'll try looking into the failures over the next couple of days, but I > can't promise anything.
Yep, I understand, I just wanted to note that it was still an outstanding item and give a quick update on why so. Anything you can look at is much appreciated. James Coleman
