UUID.compareTo broken?

[Paul Sandoz]

On Apr 8, 2014, at 9:15 PM, Mike Duigou <mike.duigou at oracle.com> wrote:
>> For the case of incorrect signed comparison is it sticking around because there is code dependent on the current broken behaviour?
> 
> Probably even if the dependence is implicit such as expecting a particular iteration order
> 
>> or do you think it would be possible to change that?
> 
> I requested fixing the compareTo long ago (back when I was a mere user of the JDK) and was told that it could not be changed.

Any more details on that?


> 
>> i have my suspicious that code dependent compareTo may not break if the total ordering changes, since many cases are likely to be for randomly generated UUIDs.
> 
> I agree that most code would probably be fine. I worry though that there will be cases that either explicitly or implicitly depend upon current ordering. Surprisingly there a lot of cases where UUIDs are created using the UUID(long, long) constructor with non-random data such as incrementing numbers:
> 
> UUID myUUID = new UUID(SOME_CONSTANT, atomicSerial.incrementAndGet());
> 

That seems a terribly broken usage of UUID for 128 bit numbers or a pair of signed 64 bit numbers :-)

Part of me thinks we should not be supporting such broken usage. Might be worth getting some usage data from grepcode or maven central.


> This usage exists, I believe, because we haven't provided direct support for MAC version UUIDs. Unfortunately the UUID(long,long) constructor does not check that the UUID constructed is even valid and usually they are not. This usage, while egregious, is common. These people would probably be impacted by changes in ordering.
> 

For correct usage the lsb long will for practical purposes always be -ve, since the msb is 1, as per the variant [1].

For time-based (MAC) UUIDs the comparison is totally broken as the time_low field bits will cycle often (~ every 3.5 mins by my calculation), so i doubt anyone would be using the comparator for such UUIDs to derive any meaningful order, for example, try this:

            UUID a = UUID.fromString("a938c470-bfc1-11e3-8a33-0800200c9a66");
            UUID b = UUID.fromString("14857ca0-bfc2-11e3-8a33-0800200c9a66");
            UUID c = UUID.fromString("d7a34910-bfc2-11e3-8a33-0800200c9a66");

            System.out.println(a.timestamp() < b.timestamp());
            System.out.println(b.timestamp() < c.timestamp());
            System.out.println(a.timestamp() < c.timestamp());

            System.out.println(a.compareTo(b));
            System.out.println(b.compareTo(c));
            System.out.println(a.compareTo(c));



>>> We could provide static methods to return appropriate comparators for version 1 and version 2 UUIDs--I've actually written them before for other projects.
>>> 
>> 
>> It would be nice to just have one compareTo that does the right thing based of the UUID types being compared.
> 
> If it were up to me only the time and DCE UUIDs would be comparable, there's no ordering relationship for other versions.

I think it is fine for random UUIDs to be comparable with each other.


> The comparators I've considered adding would only allow comparisons within the same version.
> 

Yes, although for a general comparator the primary sort key could be the version value.


>>> I also note that UUID does not currently support version 5, SHA-1, which it should.
>>> 
>>> I am hoping to do other performance work on UUID within the scope of Java 9. Adding additional Comparators would fit right in with that.
>>> 
>> 
>> OK, although it might be nice to bash this on the head sooner? as it might be possible to get into an 8u release depending on what we conclude about backwards compatibility.
> 
> Any change to the ordering of the compareTo would have to happen in a major version if at all. I am very reluctant to change this behaviour when providing alternative comparators might just be a better choice.
> 

Here is an alternative, deprecate UUID and create a new UUID class that is not broken. If we cannot fix the existing class lets just recognize it for what it is, a defective implementation for 128 bit numbers.

Paul.

[1] https://tools.ietf.org/html/rfc4122#section-4.1.1