On 11/06/2014 02:30 AM, Peter Geoghegan wrote:
Thanks for the review.
On Wed, Nov 5, 2014 at 4:33 PM, Andreas Karlsson <andr...@proxel.se> wrote:
I looked at the changes to the code. The new code is clean and there is more
code re-use and improved readability. On possible further improvement would
be to move the preparation of SortSupport to a common function since this is
done three time in the code.
The idea there is to have more direct control of sortsupport. With the
abbreviated keys patch, abbreviation occurs based on a decision made
by tuplesort.c. I can see why you'd say that, but I prefer to keep
initialization of sortsupport structs largely concentrated in
tuplesort.c, and more or less uniform regardless of the tuple-type
being sorted.
Ok, I am fine with either.
Is there any case where we should expect any greater performance
improvement?
The really compelling case is abbreviated keys - as you probably know,
there is a patch that builds on this patch, and the abbreviated keys
patch, so that B-Tree builds and CLUSTER can use abbreviated keys too.
That doesn't really have much to do with this patch, though. The
important point is that heap tuple sorting (with a query that has no
client overhead, and involves one big sort) and B-Tree index creation
both have their tuplesort as a totally dominant cost. The improvements
for each case should be quite comparable, which is good (except, as
noted in my opening e-mail, when heap/datum tuplesorting can use the
onlyKey optimization, while B-Tree/CLUSTER tuplesorting cannot).
Ah, I see.
--
Andreas Karlsson
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
