feat(parse json): add arbitrary_precision for serde_json#1656
Open
YJDoc2 wants to merge 1 commit intovectordotdev:mainfrom
Open
feat(parse json): add arbitrary_precision for serde_json#1656YJDoc2 wants to merge 1 commit intovectordotdev:mainfrom
YJDoc2 wants to merge 1 commit intovectordotdev:mainfrom
Conversation
Signed-off-by: Yashodhan Joshi <yjdoc2@gmail.com>
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.
Summary
This PR adds a non-default feature called
arbitrary_precisionin this crate, which enabls the same in theserde_jsondependency. Along with that, it also adds the correct(?) handling for serde_json's internal in-band representation of certain numbers. Also see this specifically the headerTLS Shenanigansfor more info on this that I referred.As the feature is non-default, by default it should not cause any penalty to users who do not enable it. For those who do, I have tried to make it minimally impacting.
I am opening this PR first to get initial review on if I have done this in the correct place, any discussion about motivation of the change, and if this change is something of wont-fix category etc.
Notes to reviewers -
KeyStringandValuestructs, so serde_json cannot perform its own checks for in-band values like normal. Hence we also need to add the same checks in this crateChange Type
Is this a breaking change?
How did you test this PR?
For a minimal reproduction and fix check of this issue, use the following program :
Note here that
bodyitself is a json string, andv2is a number. Now if we run it without the arbitrary precision feature (i.e. main branch) , we get output as"{\"body\":{\"v1\":\"abc\",\"v2\":{\"$serde_json::private::Number\":\"123456.123\"}}}"If we run with the feature enabled, we get the output as
"{\"body\":{\"v1\":\"abc\",\"v2\":123456.123}}"I believe second is the correct one. Also this is specific to floats, if we make the number just
123456, it works as expected on main branch as well.Does this PR include user facing changes?
our guidelines. : Will do it after first review.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here. : N/AReferences
arbitrary_precisionoption forserde_jsonfloat conversions #1503