-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Adds padding to keyrange comparison #8296
Adds padding to keyrange comparison #8296
Conversation
Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
// This means that from a keyrange perspective -80 == 00-80 == 0000-8000 == 000000-800000 | ||
// If we don't add this padding, we could run into issues when transitioning from keyranges | ||
// that use 2 bytes to 4 bytes. | ||
func addPadding(kr []byte) []byte { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like it's converting a key from a "short" version into something normalized? It might make sense to expose this as a function on the KeyRange struct or as a (KeyRange) -> KeyRange
then make that an exported function from the package.
I don't think this is a blocking comment; this PR seems safe/good to merge is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The (very reasonable) counterpoint here is that it's a bit late to start talking about a normalized keyrange format; i don't really know if it's worth trying to go down that road at this point unless this comes up again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice tests.
* We ran into some issues related to padding. This is another form of the bug fixed in here: vitessio#8296 Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
…ge-comparison Adds padding to keyrange comparison
Description
-0080
toff80-
).0000-8000 == 000000-800000
.Checklist