-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
issue-1770 - big number not returned as DecimalNode #4191
Conversation
Update NumberNodes1770Test.java Update NumberNodes1770Test.java
cd3cc18
to
1fcc82a
Compare
try { | ||
return _fromBigDecimal(ctxt, nodeFactory, p.getDecimalValue()); | ||
} catch (NumberFormatException nfe) { | ||
// fall through - BigDecimal does not support values like NaN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this makes sense -- this is an error condition and we should report an error shouldn't we?
(existing code was wrong already, I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, let me re-think -- should probably merge this as suggested but file a follow-up issue to consider making it fail as expected.
And possibly do that with configurable JsonNodeFeature
setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #4194 as follow-up
Thank you @pjfanning ! |
see #1770
also see FasterXML/jackson-core#1135
Makes the failing test
NumberNodes1770Test
passBreaks another test -
NotANumberConversionTest.testBigDecimalWithNaN
- I have added a hack to this PR to fix that test. BigDecimal does not support NaN value.