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

Strange behavior serializing BigDecimal with JSON.generate #693

Closed
mrbongiolo opened this issue Aug 12, 2021 · 6 comments
Closed

Strange behavior serializing BigDecimal with JSON.generate #693

mrbongiolo opened this issue Aug 12, 2021 · 6 comments

Comments

@mrbongiolo
Copy link
Contributor

Hi @ohler55, thanks for your amazing work on Oj! I found an "issue" in the way BigDecimal is handled when upgrading from v3.11.3 to v3.11.4 when using JSON.generate.

It's a rails project with using those configs:

Oj.optimize_rails
Oj.default_options = {
  mode: :rails,
  float_precision: 0,
  bigdecimal_as_decimal: true,
  bigdecimal_load: :bigdecimal,
  trace: true
}

When using v3.11.3:

[1] pry(main)> JSON.generate({:ticker=>"ASAI3", :price=>18.7.to_d, :rate=>-0.8.to_d})
#0:dump_compat.c:938:Oj:}: dump Hash
#0:dump_compat.c:938:Oj:}:   dump String
#0:dump_compat.c:963:Oj:{:   dump String
#0:dump_compat.c:938:Oj:}:   dump BigDecimal
#0:dump_compat.c:120:Oj:>: to_json BigDecimal
#0:      rails.c:1493:Oj:}: dump BigDecimal
#0:      rails.c:1504:Oj:{: dump BigDecimal
#0:dump_compat.c:128:Oj:<: to_json BigDecimal
#0:dump_compat.c:963:Oj:{:   dump BigDecimal
#0:dump_compat.c:938:Oj:}:   dump BigDecimal
#0:dump_compat.c:120:Oj:>: to_json BigDecimal
#0:      rails.c:1493:Oj:}: dump BigDecimal
#0:      rails.c:1504:Oj:{: dump BigDecimal
#0:dump_compat.c:128:Oj:<: to_json BigDecimal
#0:dump_compat.c:963:Oj:{:   dump BigDecimal
#0:dump_compat.c:963:Oj:{: dump Hash
=> "{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"

[3] pry(main)> {:ticker=>"ASAI3", :price=>18.7.to_d, :rate=>-0.8.to_d}.to_json
#0:      rails.c:1493:Oj:}: dump Hash
#0:      rails.c:1493:Oj:}:   dump String
#0:      rails.c:1504:Oj:{:   dump String
#0:      rails.c:1493:Oj:}:   dump BigDecimal
#0:      rails.c:1504:Oj:{:   dump BigDecimal
#0:      rails.c:1493:Oj:}:   dump BigDecimal
#0:      rails.c:1504:Oj:{:   dump BigDecimal
#0:      rails.c:1504:Oj:{: dump Hash
=> "{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"

BigDecimal values are converted to numerical values.

When using v3.11.4:

[1] pry(main)> JSON.generate({:ticker=>"ASAI3", :price=>18.7.to_d, :rate=>-0.8.to_d})
#0:dump_compat.c:939:Oj:}: dump Hash
#0:dump_compat.c:939:Oj:}:   dump String
#0:dump_compat.c:964:Oj:{:   dump String
#0:dump_compat.c:939:Oj:}:   dump BigDecimal
#0:dump_compat.c:121:Oj:>: to_json BigDecimal
#0:dump_compat.c:129:Oj:<: to_json BigDecimal
#0:dump_compat.c:964:Oj:{:   dump BigDecimal
#0:dump_compat.c:939:Oj:}:   dump BigDecimal
#0:dump_compat.c:121:Oj:>: to_json BigDecimal
#0:dump_compat.c:129:Oj:<: to_json BigDecimal
#0:dump_compat.c:964:Oj:{:   dump BigDecimal
#0:dump_compat.c:964:Oj:{: dump Hash
=> "{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":\"-0.8\"}"

[2] pry(main)> {:ticker=>"ASAI3", :price=>18.7.to_d, :rate=>-0.8.to_d}.to_json
#0:      rails.c:1491:Oj:}: dump Hash
#0:      rails.c:1491:Oj:}:   dump String
#0:      rails.c:1502:Oj:{:   dump String
#0:      rails.c:1491:Oj:}:   dump BigDecimal
#0:      rails.c:1502:Oj:{:   dump BigDecimal
#0:      rails.c:1491:Oj:}:   dump BigDecimal
#0:      rails.c:1502:Oj:{:   dump BigDecimal
#0:      rails.c:1502:Oj:{: dump Hash
=> "{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"

In this case, BigDecimal values are being converted to string values when JSON.generate is used.

I found this when some ActiveJobs (with Sidekiq as the queue adapter) started to fail, those jobs receive BigDecimal values and now those values are being processed as string when the job deserializes the arguments. As far as I could debug it seems like Sidekiq uses JSON.generate when pushing the job data to the Redis database, thus creating inconsistency.

The issue seems to be introduced on v3.11.4, but is still present on the latest release (3.13.0 atm).

Let me know if you need more information.

@ohler55
Copy link
Owner

ohler55 commented Aug 27, 2021

Sorry for the delay. When I load up rails and the JSON gem and call JSON.generate({:ticker=>"ASAI3", :price=>18.7.to_d, :rate=>-0.8.to_d}) I get the string version of the BigDecimal. That means the current version of Oj is consistent with the JSON gem or at least the current version. The JSON gem does change depending on the versions and I also note that calling {:ticker=>"ASAI3", :price=>18.7.to_d, :rate=>-0.8.to_d}.to_json also return the string version of the BigDecimals.

Without the changes to the defaults Oj behaves the same as both test cases. BigDecimals are output as strings. Setting the :bigdecimal_as_decimal option breaks compatibility as described on the Modes page of the docs (footnote 3) but it does output as a decimal. That cheat works with JSON.generate as well only if the default options are set after optimize_rails. Or at least it did for me with the latest version of Oj. Since it is a break with compatibility with the JSON gem I can't really try to "fix" it without breaking something else.

I know that doesn't solve you problem so maybe we can discuss other ways of getting what you want. One thought is not using the rails or compat modes. Is that possible?

@mrbongiolo
Copy link
Contributor Author

mrbongiolo commented Sep 16, 2021

Hi, Peter. Thanks for your attention on this issue and also sorry for my late response.

I understand that you can't change how JSON.generate works to maintain compatibility, but what I find strange is that Oj and JSON does not seem to respect the default_config when I set bigdecimal_as_decimal: true.

Here is a small script that I used to test:

require 'bundler/inline'

OJ_VERSION = '3.13.6'

puts "With OJ version (#{OJ_VERSION})"

gemfile do
  source 'https://rubygems.org'
  gem 'rails'
  gem 'oj', OJ_VERSION
end

puts "Before optimizing"

puts "Load"

p JSON.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")
p Oj.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")

puts "Generate"

p JSON.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})
p Oj.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})

Oj.optimize_rails

Oj.default_options = {
  mode: :rails,
  bigdecimal_as_decimal: true,
  bigdecimal_load: true
}

puts "After optimizing"

puts "Load"

p JSON.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")
p Oj.load("{\"ticker\":\"ASAI3\",\"price\":\"18.7\",\"rate\":-0.8}")

puts "Generate"

p JSON.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})
p Oj.generate({:ticker=>"ASAI3", :price=>18.7, :rate=>-0.8.to_d})

Results when running with v3.11.3:

With OJ version (3.11.3)
Before optimizing
Load
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8}
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8}
Generate
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
After optimizing
Load
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8e0}
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8e0}
Generate
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":-0.8}"

Results when running with latest (v3.13.6):

With OJ version (3.13.6)
Before optimizing
Load
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8}
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8}
Generate
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
After optimizing
Load
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8e0}
{"ticker"=>"ASAI3", "price"=>"18.7", "rate"=>-0.8e0}
Generate
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"
"{\"ticker\":\"ASAI3\",\"price\":18.7,\"rate\":\"-0.8\"}"

When using the latest version Oj.generate and JSON.generate they are still outputting the decimal values as strings, even after setting default_options to bigdecimal_as_decimal: true. Float values seem to be working correcting, as is shown with the 18.7 value.
Should the optimize_rails and default_options be set in a different way or are they supposed to work just like that? Because AFAIK when setting bigdecimal_as_decimal: true it should output those values as decimals as it does on version 3.11.3.
I do understand that setting this option breaks compatibility, but right now it seems like the option is ignored or something.

If you need any more information just let me know.

@ohler55
Copy link
Owner

ohler55 commented Sep 17, 2021

The question to ask if you are using Oj mimicking the JSON gem is whether the JSON gem has that option. Since it does not it will not be honored. The http://www.ohler.com/oj/doc/file.Modes.html shows what is supported in each mode.

I just noticed yard didn't generate the table correctly. I'll clean that up tonight.

@ohler55
Copy link
Owner

ohler55 commented Sep 17, 2021

Table should look okay now. I really can't support a deviation from the JSON gem behaviour for the :compat and :rails modes as there will be others that complain for that deviation. My best suggestion is to use :custom mode so you can tune Oj to what you want.

@mrbongiolo
Copy link
Contributor Author

mrbongiolo commented Sep 18, 2021 via email

@ohler55
Copy link
Owner

ohler55 commented Sep 18, 2021

Sadly the interaction between the JSON gem and Rails is extremely complex and inconsistent. You might notice that a very high percentage of issues are around getting the Oj behaviour to match under different situations.

As for the rails mode, rails also has the active support generator and it monkey patches the same types that the JSON gem does. If you find a combination where Oj is different than either rails or the JSON gem I'll take a look. Sure hope you don't though. There are so many special cases as it is.

mrbongiolo added a commit to mrbongiolo/oj that referenced this issue Oct 14, 2021
The option `:bigdecimal_as_decimal` is not available in Rails Mode (as discussed here ohler55#693), as the original JSON and Rails implementation does not have a way to "force" bigdecimal values to be encoded as numbers in json.
ohler55 pushed a commit that referenced this issue Oct 28, 2021
* Update usage of :bigdecimal_as_decimal in Rails Mode

The option `:bigdecimal_as_decimal` is not available in Rails Mode (as discussed here #693), as the original JSON and Rails implementation does not have a way to "force" bigdecimal values to be encoded as numbers in json.

* Update docs for :bigdecimal_as_decimal in Rails Mode

* Use a more welcoming message
@ohler55 ohler55 closed this as completed Dec 4, 2023
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