From: | James Coleman <jtc331(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
Cc: | Peter Geoghegan <pg(at)bowt(dot)ie>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, Shaun Thomas <shaun(dot)thomas(at)2ndquadrant(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [PATCH] Incremental sort (was: PoC: Partial sort) |
Date: | 2019-07-07 13:01:43 |
Message-ID: | CAAaqYe9gvGoo=wZMqMjmOrMu2m_cPyPEsbxZnsMOBSDn=VN14Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Jul 7, 2019 at 8:34 AM Tomas Vondra
<tomas(dot)vondra(at)2ndquadrant(dot)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(dot)vondra(at)2ndquadrant(dot)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.
> >
>
> I wanted to investigate this today, but I can't reprodure it. How are
> you building and running the regression tests?
>
> Attached is a patch adding the incremental sort below gather merge, and
> also tweaking the costing. But that's mostly for and better planning
> decisions, I don't get any pathkey errors even with the first patch.
On 12be7f7f997debe4e05e84b69c03ecf7051b1d79 (the last patch I sent,
which is based on top of 5683b34956b4e8da9dccadc2e3a53b86104ebb33), I
did this:
patch -p1 < ~/Downloads/parallel-incremental-sort.patch
<rebuild> (FWIW I configure with ./configure
--prefix=$HOME/postgresql-test --enable-cassert --enable-debug
--enable-depend CFLAGS="-ggdb -Og -g3 -fno-omit-frame-pointer
-DOPTIMIZER_DEBUG")
make check-world
And I get the attached regression failures.
James Coleman
Attachment | Content-Type | Size |
---|---|---|
regression.diffs | application/octet-stream | 34.3 KB |
regression.out | application/octet-stream | 12.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2019-07-07 13:38:51 | Re: (select query)/relation as first class citizen |
Previous Message | Roman Pekar | 2019-07-07 12:54:26 | (select query)/relation as first class citizen |