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.

Sign up for GitHub

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

Jump to bottom

Serialization of LiteralKeyExpression casts long value to floats #756

Closed
nschiefer opened this issue Oct 21, 2019 · 1 comment · Fixed by #757
Closed

Serialization of LiteralKeyExpression casts long value to floats #756

nschiefer opened this issue Oct 21, 2019 · 1 comment · Fixed by #757
Assignees
@nschiefer
Labels
bug Something isn't working

Comments

@nschiefer
Copy link
Contributor

When we create a LiteralKeyExpression using Java objects (instead of passing an existing Protobuf), we use the LiteraKeyExpression.toProtoValue() method to create a serialized form, which is used (among other things) for the LiteralKeyExpressions equals() method.

Currently, all numeric types, including integers, are converted to floating point values in this serialization.
@nschiefer nschiefer self-assigned this Oct 21, 2019
@nschiefer nschiefer added the bug Something isn't working label Oct 21, 2019
@nschiefer
Copy link
Contributor Author

A similar (but less subtle) issue shows up with LiteralKeyExpressions of byte arrays. When we deserialize the serialized form, we produce a LiteralKeyExpression of ByteString rather than byte[].

@nschiefer nschiefer mentioned this issue Oct 21, 2019
Merged
nschiefer added a commit to nschiefer/fdb-record-layer that referenced this issue Oct 21, 2019
@nschiefer
Fixes FoundationDB#756: Serialization of LiteralKeyExpression can be …
6ac0835 
…incorrect.

Previously, serializing certain LiteralKeyExpressions (of type Long
or byte[]) would produce a Protobuf that, when deserialized, would
not contain a value of a different type (Float or ByteString,
respectively). This fixes FoundationDB#756 and adds some tests that should
give us more confidence that the serialization is correct.
@MMcM MMcM closed this as completed in #757 Oct 22, 2019
MMcM added a commit that referenced this issue Oct 22, 2019
@MMcM
Merge pull request #757 from nschiefer/fix-literal-key-expression-proto
e5ba13b 
Fixes #756: Serialization of LiteralKeyExpression can be incorrect.
@MMcM MMcM mentioned this issue Apr 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nschiefer nschiefer

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #756: Serialization of LiteralKeyExpression can be incorrect. nschiefer/fdb-record-layer
1 participant
@nschiefer