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

Fix issue with BigInteger handling #31

Merged
merged 3 commits into from
Apr 9, 2023

Conversation

pjfanning
Copy link
Member

  • I added the test case and it failed
  • I debugged the code and found that org.glassfish.json.JsonNumberImpl uses BigDecimals under the hood so there is no perf benefit to trying to use doubles (and the risk with the doubles losing precision and truncating big numbers)
  • what happened in my test is the Jackson code uses toDouble and that evals to Double.Infinity and things get worse from there

@cowtowncoder
Copy link
Member

SGTM/LGTM: we'd need similar for jakarta-jsonp too I think. Will merge this first.

@cowtowncoder cowtowncoder merged commit 6394c51 into FasterXML:2.15 Apr 9, 2023
@cowtowncoder cowtowncoder changed the title [jsonp] fix issue with big int handing Fix issue with big int handing Apr 9, 2023
@cowtowncoder cowtowncoder added the jsr-353 Issue related to JSR-353/JSONP datatype module label Apr 9, 2023
@cowtowncoder cowtowncoder changed the title Fix issue with big int handing Fix issue with BigInteger handling Apr 9, 2023
cowtowncoder added a commit that referenced this pull request Apr 9, 2023
@cowtowncoder cowtowncoder added this to the 2.15.0-rc3 milestone Apr 9, 2023
@pjfanning pjfanning deleted the jsonp-big-int branch April 9, 2023 08:33
@pjfanning
Copy link
Member Author

@cowtowncoder could you merge #32 and #33 before the rc3 release?

@cowtowncoder
Copy link
Member

@pjfanning Done. I am still trying to close the loop on a few things; will sync up with you before proceeding -- won't be happening this weekend as things are (rc3 release that is)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jsr-353 Issue related to JSR-353/JSONP datatype module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants