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

Must compare strings by codepoint instead of codeunit #2113

Open
erights opened this issue Mar 3, 2024 · 0 comments · May be fixed by #2008
Open

Must compare strings by codepoint instead of codeunit #2113

erights opened this issue Mar 3, 2024 · 0 comments · May be fixed by #2008
Assignees
Labels
bug Something isn't working

Comments

@erights
Copy link
Contributor

erights commented Mar 3, 2024

Describe the bug

Currently, our compareRank and compareKeys applied to strings compares them in sort order using JavaScript's < operator as applied to strings. Unfortunately, JS < compares strings according to their lexicographic UTF16 code unit order. (This is preserved on XS by lexicographic comparison of bytes in a CESU-8 encoding of the string.)

However, with our agreement, ocapn has standardized on using the (much more semantically sensible!) Unicode lexicographic codepoint order (which would be preserved by a proper UTF-8 encoding of well formed strings. See #1739 ).

Indeed #2008 "solves" the immediate problem regarding compareRank and compareKeys. However, there are plenty of places where we still sort strings by their implicit sort order. Worse, we don't know how much data we have already stored on chain organized by the wrong sort order. Nor do we have any practical plans for how to find or fix that data. This needs design.

@erights erights added the bug Something isn't working label Mar 3, 2024
@erights erights self-assigned this Mar 3, 2024
@erights erights linked a pull request Mar 3, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant