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

Binding numeric values can BigDecimal lose precision #1315

Closed
asnare opened this issue Jul 27, 2016 · 2 comments
Closed

Binding numeric values can BigDecimal lose precision #1315

asnare opened this issue Jul 27, 2016 · 2 comments
Milestone

Comments

@asnare
Copy link

asnare commented Jul 27, 2016

It looks like commit c809c0c (intended to fix #1028) introduced an issue where numeric values lose precision prior to being returned as BigDecimal instances. (Feature USE_BIG_DECIMAL_FOR_FLOATS enables this; given that it's disabled for performance by default, the reason to enable it is when you specifically want to avoid precision loss.)

This issue is still present in 2.8.1 (which is where I encountered it).

Some initial discussion took place as comments on the commit. I don't know enough about the contracts involved in the parsing state, but some things that caught my eye:

  • Retrieving the parser value as a Double is where the precision loss occurs.
  • The fix was intended to deal with returning NaN/Inf, which BigDecimal doesn't support. However these aren't legal values in JSON, and there's a ALLOW_NON_NUMERIC_NUMBERS feature to allow this as extension. (Maybe the feature-check is somewhere else? I haven't looked.)
@cowtowncoder
Copy link
Member

Thank you for reporting this. I agree in code being suspicious, but it would be great it if there was an easy reproduction for verification purposes.

@cowtowncoder cowtowncoder added this to the 2.2 milestone Aug 22, 2016
@cowtowncoder
Copy link
Member

Fixed.

Handling is bit sub-optimal, in that I think decoding from textual form to numeric may need to be done twice now; ideally should be able to see if NaN value is encountered without such parsing.
I may file an RFE for streaming parser to indicate this state to avoid parsing twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants