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

Fix key-sorting bugs. #733

Open
wants to merge 2 commits into
base: v2
Choose a base branch
from

Conversation

tomclegg
Copy link

Existing key sorting is nondeterministic because (keyList)Less() returns contradictory results.

  • Less("7a","8a") returns true: 7 < 8
  • Less("8a","77") returns true: 8 < 77
  • Less("77","7a") returns true: the first char is equal ("continue"), then "7" < "a" because "a" is a letter

(This has been reported previously on a closed issue.)

There are other surprising behaviors, too:

  • Numbers that overflow int64 are sorted strangely ("9223372036854775808" < "9223372036854775807")
  • Non-ASCII digits are sorted strangely ("\u0666" is sorted as if it's "1590")

This commit fixes the above issues, adds some keys in the sorting test to expose some edge cases, and adds a loop in the test to more reliably catch nondeterministic sorting.

@tomclegg
Copy link
Author

Existing unmerged PR #439 addresses the nondeterministic behavior but not the int64/non-ascii issues.

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

Successfully merging this pull request may close these issues.

None yet

1 participant