Ord instance appears to be broken

[lgastako]

λ> accountId0
Id {unId = 00000000000000000000000000}
λ> accountId1
Id {unId = 00000000000000000000000001}
λ> hash accountId0
5618888146721150863
λ> hash accountId1
5618887047209522684
λ> accountId0 == accountId1
False
λ> -- So far so good, but...
λ> accountId0 < accountId1
False
λ> -- and...
λ> accountId0 > accountId1
False

[TheInnerLight]

I Just spotted this too, this is because:

instance Ord ULID where
    compare (ULID ts1 _) (ULID ts2 _) = compare ts1 ts2

This means the random component is ignored and breaks lexicographic ordering.

I don't see any reason that the ULIDRandom type shouldn't derive Ord and then ULID can also derive Ord.

[ad-si]

I know it seems weird, but I'd argue that this is the correct behavior: It's not exactly the same thing, but neither one happens before the other one as they have the same timestamp.

Can you give an example for code that should work, but doesn't because of this behavior?

[TheInnerLight]

My use case that is broken involves putting ULIDs in a Map or Set. The result is that not all the ULIDs will be stored in the map, only the first one with a unique timestamp because the data structure will assume that the ULIDs with the same timestamp but different random components are actually equal.

This is an issue with any data type where the definitions of Ord and Eq don't align. In the docs (https://hackage.haskell.org/package/base-4.20.0.1/docs/Data-Ord.html) property 6 (x == y = compare x y == EQ) is supposed to hold for all Ord instances but it doesn't here.

Ord instances are for types with total orders and that indeed should be the case as lexicographic ordering is a core component of the ULID data type. Indeed, one of the key motivating use cases for ULIDs is their sequential nature for efficient database insert and indexing.

The way the ordering is currently implemented in this library is currently not lexicographic and is a partial order rather than the total order implied by the Ord instance.

[ad-si]

Fixed in 3030780