Re: [PATCH] Incremental sort (was: PoC: Partial sort)

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-04 13:29:49
Message-ID: CAAaqYe8PVSO3dzysZZ-RB8HRMAZ_JMOPwW--veY9ZEuTCcbe8A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

James Coleman

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kumar, Pawan (Nokia - IN/Bangalore) 2019-07-04 13:37:01 duplicate key entries for primary key -- need urgent help
Previous Message James Coleman 2019-07-04 13:25:08 Re: [PATCH] Incremental sort (was: PoC: Partial sort)