On Tue, Oct 2, 2018 at 10:55 AM Andrew Dunstan <andrew.duns...@2ndquadrant.com> wrote: > On 10/01/2018 12:50 PM, Tom Lane wrote: > > Andres Freund <and...@anarazel.de> writes: > >> On 2018-10-01 12:13:57 -0400, Tom Lane wrote: > >>> Yeah. So our choices are > >>> > >>> (1) Retain the current restriction on what sort comparators can > >>> produce. Find all the places where memcmp's result is returned > >>> directly, and fix them. (I wonder if strcmp has same issue.) > >>> > >>> (2) Drop the restriction. This'd require at least changing the > >>> DESC correction, and maybe other things. I'm not sure what the > >>> odds would be of finding everyplace we need to check. > >>> > >>> Neither one is sounding very pleasant, or maintainable. > >> (2) seems more maintainable to me (or perhaps less unmaintainable). It's > >> infrastructure, rather than every datatype + support out there... > > I guess we could set up some testing infrastructure: hack int4cmp > > and/or a couple other popular comparators so that they *always* > > return INT_MIN, 0, or INT_MAX, and then see what falls over. > > > > I'm fairly sure that btree, as well as the sort code proper, > > has got an issue here. > > > > > > > I agree option 2 seems less unmaintainable. (Nice use of litotes there?)
+1 for option 2. It seems to me that it should ideally be the job of the code that is negating the value to deal with this edge case. I see that the restriction is clearly documented at the top of src/backend/access/nbtree/nbtcompare.c even though it directly returns strncmp() results. This was quite a surprising thread. -- Thomas Munro http://www.enterprisedb.com