On Thu, Jan 19, 2012 at 1:47 PM, Peter Geoghegan <pe...@2ndquadrant.com> wrote: > Thoughts?
I generated some random data using this query: create table nodups (g) as select (g%10)*1000+g/10 from generate_series(1,10000) g; And then used pgbench to repeatedly sort it using this query: select * from nodups order by g offset 10001; The patch came out about 28% faster than master. Admittedly, that's with no client overhead, but still: not bad. I don't like the API you've designed, though: as you have it, PrepareSortSupportFromOrderingOp does this after calling the sort support function: + ssup->usable_compar = ResolveComparatorProper(sortFunction); I think it would be better to simply have the sort support functions set usable_compar themselves. That would allow any user-defined functions that happen to have the same binary representation and comparison rules as one of the types for which we supply a custom qsort() to use initialize it to still make use of the optimization. There's no real reason to have a separate switch to decide how to initialize that field: the sort support trampoline already does that, and I don't see any reason to introduce a second way of doing the same thing. I am also a little unhappy that we have to duplicate code the fastcmp functions from nbtcompare.c in builtins.h. Wouldn't it make more sense to declare those functions as inline in nbtcompare.c, and then call the qsort-generating macro from that file? There were a couple of comment updates in tuplesort.c that looked independent from the reset of the patch, so I've committed those separately. I also committed your change to downgrade the belt-and-suspenders check for self-comparison to an assert, with some rewording of your proposed comment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers