On Sun, Sep 11, 2016 at 6:28 AM, Heikki Linnakangas <hlinn...@iki.fi> wrote: > * I renamed "tuplesort_heap_siftup()" to "tuplesort_delete_top()". I realize > that this is controversial, per the discussion on the "Is > tuplesort_heap_siftup() a misnomer?" thread. However, now that we have a new > function, "tuplesort_heap_replace_top()", which is exactly the same > algorithm as the "delete_top()" algorithm, calling one of them "siftup" > became just too confusing.
I feel pretty strongly that this was the correct decision. I would have gone further, and removed any mention of "Sift up", but you can't win them all. > * Instead of "root_displace", I used the name "replace_top", and > "delete_top" for the old siftup function. Because we use "top" to refer to > memtuples[0] more commonly than "root", in the existing comments. Fine by me. > * I shared the code between the delete-top and replace-top. Delete-top now > calls the replace-top function, with the last element of the heap. Both > functions have the same signature, i.e. they both take the checkIndex > argument. Peter's patch left that out for the "replace" function, on > performance grounds, but if that's worthwhile, that seems like a separate > optimization. Might be worth benchmarking that separately, but I didn't want > to conflate that with this patch. Okay. > * I replaced a few more siftup+insert calls with the new combined > replace-top operation. Because why not. I suppose that the consistency has value, from a code clarity standpoint. > Thanks for the patch, Peter, and thanks for the review, Claudio! Thanks Heikki! -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
