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

[Internal] JSON Binary Encoding: Adds support for writing unsigned 64-bit integer values #4883

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sboshra
Copy link
Contributor

@sboshra sboshra commented Nov 11, 2024

Description

This PR adds support for serializing unsigned Int64 values in Binary encoding, ensuring that values exceeding the maximum signed Int64 limit are preserved without conversion to floating-point doubles. This enhancement enables full-fidelity roundtrips between Text and Binary encoding formats. For values outside the signed Int64 range, retrieval from either the JSON Reader or Navigator (across all encoding formats) will return them as floating-point doubles.

Type of change

  • New feature (non-breaking change which adds functionality)

Closing issues

None

@adityasa
Copy link
Contributor

adityasa commented Nov 11, 2024

Does this change contain any bugfixes (either by themselves or for previous commit)? Asking to see if it should be included in the release that's in progress. #Resolved

- Added a new abstract method, `TryGetUInt64Value`, to both the `JsonReader` and `JsonNavigator` base classes. This method attempts to retrieve a number as an unsigned 64-bit integer.
- Updated `JsonReader.WriteCurrentToken` and `JsonNavigator.WriteNode` to first attempt to retrieve the number as a UInt64 and write it as such if successful; otherwise, they fall back to writing it as a `Number64` value.
- Corrected a typo in `JsonWriter.ForceRewriteRawJsonValue`, where the check for the Boolean value `enableNumberArrays` was mistakenly replaced by `enableEncodedStrings`.
- Renamed `JsonNodeType.Number64` to `JsonNodeType.Number`.
- Similarly, renamed `IJsonNavigator.GetNumber64Value` to `GetNumberValue`.
- Enhanced JSON round-trip test validation to allow strict comparison, ensuring that the expected and actual buffers are identical.
@sboshra
Copy link
Contributor Author

sboshra commented Nov 12, 2024

It mainly adds support of reading/writing UInt64 values, which is soon to be needed once writing such values is enabled in the backend.


In reply to: 2468664283

Copy link
Contributor

@adityasa adityasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@neildsh neildsh added QUERY auto-merge Enables automation to merge PRs labels Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Enables automation to merge PRs QUERY
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants