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

Decimal should be unquoted in produced JSON #71

Closed
kamenlitchev opened this issue Mar 29, 2019 · 8 comments
Closed

Decimal should be unquoted in produced JSON #71

kamenlitchev opened this issue Mar 29, 2019 · 8 comments

Comments

@kamenlitchev
Copy link

kamenlitchev commented Mar 29, 2019

Decimal should result in unquoted values in the produced JSON. Any Decimal type should be aligned with the behaviour of Integer and Float.

Currently we have the following:

Jason.encode! %{decimal: Decimal.new("1.23")}

> {"decimal":"1.23"}

Can we have Jason not put quotes around the value and produce the following JSON instead?

{"decimal":1.23}

Given that Decimal is in fact a number, and none of the ECMA/RFC standards define number (decimal/integer) boundaries, it is strange to map it to a string.

@ericmj
Copy link

ericmj commented Mar 29, 2019

You could do this with:

defimpl Jason.Encoder, for: Decimal do
  def encode(decimal, _opts) do
    Decimal.to_string(decimal, :normal)
  end
end

Also note that Jason.decode!(Jason.encode!(decimal)) != decimal since Jason will decode numbers to floats or integers.

@kamenlitchev
Copy link
Author

Thank you!

As for the decode - that I fully understand :)

@kamenlitchev
Copy link
Author

Hey, sorry,

Did so, but it seems that the customer implementation is not being called. Jason.Encode.encode would call it, but not Jason.encode!. Any advise?

@ericmj
Copy link

ericmj commented Mar 29, 2019

It seems like Jason bypasses the protocols for the types it supports out of the box. So my proposal doesn't work, sorry.

@kamenlitchev
Copy link
Author

@michalmuskala, I am willing to help here - I really like to see an option to:

  • encode Decimal as unquoted string (similar to float)
  • decode floats to Decimal (instead of float)

But I am not sure how to proceed. Since all encoder/decoder options are internally passed as function arguments, introducing new explicit option for Decimal handling means to litter the code everywhere.

Another approach is to put Application-level (may hit float/decimal performance) or Jason module level flag. Then, again, I saw nowhere such a thing and it may be out of sync with the general project structure.

Do you have any suggestions?

@michalmuskala
Copy link
Owner

michalmuskala commented Apr 1, 2019

This encoding is intentional. The Decimal data type is about exact representation of a decimal number - something that cannot be done by a JSON number type, because they are not exact. Encoding decimals as floats would be incorrect and prone to extremely subtle errors - for example Decimal.new("9007199254740993.0") if encoded as a straight up JSON number, would be decoded in JavaScript (and most languages) incorrectly as 9007199254740992.0 since 9007199254740993 is not representable by a float.

Encoding decimals as strings is the only encoding that would not downgrade decimal precision back to the float precision - the exact reason for using decimals instead of floats in the first place.

If the loss of precision is not an issue - you might as well use floats in the first place.

Because of those reasons, I do not plan to merge any changes in this area at the moment. Thank you for raising the issue.

@kamenlitchev
Copy link
Author

Hey, thanks for the response! I do understand that point - compatibility. Thanks :)

@azizk
Copy link
Contributor

azizk commented Jun 8, 2024

Hi @michalmuskala!

Recently I've been implementing parts of the OpenRTB protocol and appreciate that Jason supports decoding JSON floats as Decimals. However, I was surprised to find that it converts them right back to string literals, thereby breaking the type specifications of the protocol!

This encoding is intentional. The Decimal data type is about exact representation of a decimal number - something that cannot be done by a JSON number type, because they are not exact.

We know in its pure form a JSON number is as exact as can be before it's decoded. It only matters what the decoder does.

Encoding decimals as floats would be incorrect and prone to extremely subtle errors - for example Decimal.new("9007199254740993.0") if encoded as a straight up JSON number, would be decoded in JavaScript (and most languages) incorrectly as 9007199254740992.0 since 9007199254740993 is not representable by a float.

That's admittedly true for JavaScript, but it's not the only consumer of JSON data. It's the responsibility of the consuming code to use proper decoding libraries. For instance, if we communicated with a Java server it would have to use BigDecimal to decode our data.

Encoding decimals as strings is the only encoding that would not downgrade decimal precision back to the float precision - the exact reason for using decimals instead of floats in the first place.

That's not quite correct. It depends on the consuming parser/language.

If the loss of precision is not an issue - you might as well use floats in the first place.

Precision is indeed an issue, and sometimes the JSON response must contain JSON floats again according to the requirements.

It's strange to me that we should send back Decimals as string literals by default, even though the request data contained JSON floats. Even if the data comes from a database and not a client I'd say it makes sense to keep the same types in the resulting encoded data.

Because of those reasons, I do not plan to merge any changes in this area at the moment. Thank you for raising the issue.

I hope you will reconsider! While changing the default behaviour would be a breaking change, providing an encoding option would satisfy both use cases.

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

4 participants