Robert Haas <robertmh...@gmail.com> writes: > On further review, this is definitely the way to go: it's a > straight-up win. The isnull array is never more than one element in > length, so testing the single element is quite trivial. The > attached, revised patch provides a modest but useful speedup for both > hash and btree index builds.
> Anyone see any reason NOT to do this? So far it looks like a > slam-dunk from here. If index_form_tuple leaks any memory in the sort context, this would be not so much a slam-dunk win as a complete disaster. I'm not sure that no-leak is a safe assumption, especially in cases where we do toast compression of the index datums. (In fact, I'm pretty sure that the reason it was coded like this originally was exactly to avoid that assumption.) Assuming that you inspect the code and determine it's safe, the patch had better decorate index_form_tuple with dire warnings about not leaking memory; because even if it's safe today, somebody might break it tomorrow. On a micro-optimization level, it might be worth passing the TID as ItemPointer not ItemPointerData (ie, pass a pointer until we get to the point of actually inserting the TID into the index tuple). I'm not sure that copying odd-size structs should be assumed to be efficient. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers