Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort order does not respect millisecond ordering #22

Closed
owst opened this issue Sep 3, 2020 · 1 comment
Closed

Sort order does not respect millisecond ordering #22

owst opened this issue Sep 3, 2020 · 1 comment

Comments

@owst
Copy link
Collaborator

owst commented Sep 3, 2020

As I read the spec in this repo, the lexicographical sort ordering of ULIDs should respect the timestamp ordering to millisecond precision. However, this does not appear to always be the case:

require 'ulid'

times = Array.new(5) { |i| Time.new(2020, 9, 3, 13, 9, Rational(i, 10**3)) }

puts 'Times:'
puts times.map { |t| t.strftime('%F %T.%N') }

ulids = times.map { |t| ULID.generate(t) }
puts 'ULIDs:'
puts ulids

sorted = ulids.sort
puts 'Sorted:'
puts sorted

puts "Sort respects time? #{ulids == sorted}"

gives

Times:
2020-09-03 13:09:00.000000000
2020-09-03 13:09:00.001000000
2020-09-03 13:09:00.002000000
2020-09-03 13:09:00.003000000
2020-09-03 13:09:00.004000000
ULIDs:
01EH9XXEV0KCZV2TBC86EGT900
01EH9XXEV1NNSE20T5E9PDH4Q9
01EH9XXEV1HSVWPK0E776QR1ZX
01EH9XXEV3XYZH7SVCH8684MVV
01EH9XXEV4205AFCAVKAT9YH2V
Sorted:
01EH9XXEV0KCZV2TBC86EGT900
01EH9XXEV1HSVWPK0E776QR1ZX
01EH9XXEV1NNSE20T5E9PDH4Q9
01EH9XXEV3XYZH7SVCH8684MVV
01EH9XXEV4205AFCAVKAT9YH2V
Sort respects time? false

The problem being that (in this example run) that 01EH9XXEV1NNSE20T5E9PDH4Q9 sorts after 01EH9XXEV1HSVWPK0E776QR1ZX, despite the timestamp used to generate the former being 1 millisecond before that of the latter. Looking at the timestamp portion of the two ULIDs, they are the same: 01EH9XXEV1, which means both 2020-09-03 13:09:00.001 and 2020-09-03 13:09:00.002map to the same ULID prefix.

rafaelsales added a commit that referenced this issue Mar 1, 2021
) (#23)

* Ensure ULID order respects timestamp order to millisecond precision (#22)

* fixup! Ensure ULID order respects timestamp order to millisecond precision (#22)

* Update lib/ulid/generator.rb

Thanks to @kachick

Co-authored-by: Kenichi Kamiya <kachick1@gmail.com>

Co-authored-by: Kenichi Kamiya <kachick1@gmail.com>
Co-authored-by: Rafael Sales <rafaelcds@gmail.com>
@owst
Copy link
Collaborator Author

owst commented Mar 3, 2021

Fixed by #23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant