Remove dubious micro-optimization in ckpt_buforder_comparator().
authorTom Lane
Wed, 10 Jan 2018 20:50:54 +0000 (15:50 -0500)
committerTom Lane
Wed, 10 Jan 2018 20:50:54 +0000 (15:50 -0500)
commitc24a908d04806c34c3e5dda42d39bb5ca1fc5ffb
tree166ff74113070828bf83259603266010bba32750
parent01268386665f4a59c2fa9236f448bf3caf8c3b0b
Remove dubious micro-optimization in ckpt_buforder_comparator().

It seems incorrect to assume that the list of CkptSortItems can never
contain duplicate page numbers: concurrent activity could result in some
page getting dropped from a low-numbered buffer and later loaded into a
high-numbered buffer while BufferSync is scanning the buffer pool.
If that happened, the comparator would give self-inconsistent results,
potentially confusing qsort().  Saving one comparison step is not worth
possibly getting the sort wrong.

So far as I can tell, nothing would actually go wrong given our current
implementation of qsort().  It might get a bit slower than expected
if there were a large number of duplicates of one value, but that's
surely a probability-epsilon case.  Still, the comment is wrong,
and if we ever switched to another sort implementation it might be
less forgiving.

In passing, avoid casting away const-ness of the argument pointers;
I've not seen any compiler complaints from that, but it seems likely
that some compilers would not like it.

Back-patch to 9.6 where this code came in, just in case I've underestimated
the possible consequences.

Discussion: https://postgr.es/m/18437.1515607610@sss.pgh.pa.us
src/backend/storage/buffer/bufmgr.c