Fix encoding of int64/bool keys of maps in encoder#1653
Closed
reebalazs wants to merge 3 commits into
Closed
Conversation
The int64 writer cannot handle the hash directly, because it interprets it as a string representation of a Long number, instead of a hash. Explicit conversion from hash to long is needed before writing.
Exactly the same issue with bool key types as well. Extend the fix to cover this case too.
Author
|
Exactly same case with a bool key type. Fix extended to cover this case as well. |
angadn
approved these changes
Nov 2, 2021
The fix has to be extended to all int64 key types.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1652 .
The int64 writer cannot handle the hash directly, because it interprets
it as a string representation of a Long number, instead of a hash.
This is obviously wrong, because the input at this point always contains the longbit hashes already. So the int64() writer fails inevitably.
I believe that the simplest solution is: Explicit conversion from hash to long is needed before writing.
Again (just like with the other PR I submitted) I believe that the tests are missing for the generated encode/decode functions. If there were such tests, it would be fairly straightforward to write a test case that exercises this problem, and proves that the fix, in fact, works.