From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: James Coleman <jtc331@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: Simon Riggs <simon@2ndquadrant.com>
Cc: Shaun Thomas <shaun.thomas@2ndquadrant.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Cc: Andreas Karlsson <andreas@proxel.se>
Subject: Re: [PATCH] Incremental sort (was: PoC: Partial sort)
Date: Sun, 5 Apr 2020 15:12:27 +0200
Message-ID: <20200405131227.zit6a5w3fv5qq6fh@development> (raw)
In-Reply-To: <20200405130110.2bdxaz4ed5jx2eae@development>
References: <CAAaqYe99B9cRLU+FWJudNx-6O4TgnWYCrRtsv2NcUs4TzG5qrw@mail.gmail.com>
<CAAaqYe953TA0Pj+08jEBgK_=5zMCJO-vgYnh4n_Me71ZdzDJLg@mail.gmail.com>
<20200401214215.ut6b72e3zlbm33w2@development>
<CAAaqYe8fWa8pBUStzW-4JocF+d3pFBVLFmb9rtcD0m12NskBJw@mail.gmail.com>
<20200402024746.5wlwk6q7eiv5faux@development>
<CAAaqYe90h3F5NbgtNJAMrXAiGZwML8=_E_qs4MCyYtqea+5Hhg@mail.gmail.com>
<20200403002028.kp3rxncmwvsui6xz@development>
<CAAaqYe--jV9pHrX5NRw9R2QGG+nRuPPKguzHhKZwhp6XPOiyuQ@mail.gmail.com>
<CAAaqYe9XO7ZKrpVkrz2e0ja=eeFCX0Wqpmc7r23R84JU9uCLxQ@mail.gmail.com>
<20200405130110.2bdxaz4ed5jx2eae@development>
On Sun, Apr 05, 2020 at 03:01:10PM +0200, Tomas Vondra wrote:
>On Thu, Apr 02, 2020 at 09:40:45PM -0400, James Coleman wrote:
>>On Thu, Apr 2, 2020 at 8:46 PM James Coleman <jtc331@gmail.com> wrote:
>>>
>>>On Thu, Apr 2, 2020 at 8:20 PM Tomas Vondra
>>><tomas.vondra@2ndquadrant.com> wrote:
>>>> ...
>>>> 5) Overall, I think the costing is OK. I'm sure we'll find cases that
>>>> will need improvements, but that's fine. However, we now have
>>>>
>>>> - cost_tuplesort (used to be cost_sort)
>>>> - cost_full_sort
>>>> - cost_incremental_sort
>>>> - cost_sort
>>>>
>>>> I find it a bit confusing that we have cost_sort and cost_full_sort. Why
>>>> don't we just keep using the dummy path in label_sort_with_costsize?
>>>> That seems to be the only external caller outside costsize.c. Then we
>>>> could either make cost_full_sort static or get rid of it entirely.
>>>
>>>This another area of the patch I haven't really modified.
>>
>>See attached for a cleanup of this; it removed cost_fullsort so
>>label_sort_with_costsize is back to how it was.
>>
>>I've directly merged this into the patch series; if you'd like to see
>>the diff I can send that along.
>>
>
>Thanks. Attached is v54 of the patch, with some minor changes. The main
>two changes are in add_partial_path_precheck(), firstly to also consider
>startup_cost, as discussed before. The second change (in 0003) is a bit
>of an experiment to make add_partial_precheck() cheaper by only calling
>compare_pathkeys after checking the costs first (which should be cheaper
>than the function call). add_path_precheck already does it in that order
>anyway.
>
Oh, I forgot to mention a change in add_partial_path - I've removed the
reference/dependency on enable_incrementalsort. It seemed rather ugly,
and the results without it seem fine (I'm benchmarking only the case
with incremental sort enabled anyway). I also plan to look at the other
optimization we bolted on last week, i.e. checking length of pathkeys.
I'll see if that actually makes measurable difference.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, jtc331@gmail.com, tgl@sss.pgh.pa.us, alvherre@2ndquadrant.com, michael@paquier.xyz, rafia.pghackers@gmail.com, pg@bowt.ie, simon@2ndquadrant.com, shaun.thomas@2ndquadrant.com, 9erthalion6@gmail.com, a.korotkov@postgrespro.ru, pgsql-hackers@lists.postgresql.org, andreas@proxel.se
Subject: Re: [PATCH] Incremental sort (was: PoC: Partial sort)
In-Reply-To: <20200405131227.zit6a5w3fv5qq6fh@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox