search

FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
310 stars 133 forks source link

Incorrect `BigDecimal` fraction representation (2.9 -> 2.10 change) #139

Closed wlukowicz closed 5 years ago

wlukowicz commented 6 years ago

According to the spec, decimal fractions (tag 4) of value m 10^e should be represented as a tagged array containing the exponent e and the mantissa m. However, Jackson writes BigDecimals as tagged arrays containing their scale and unscaled value. The difference being that the value of BigDecimal is unscaled value 10^-scale, which means the scale is the opposite of the exponent e.

The spec gives an example how 273.15 should be represented:

C4             -- Tag 4
   82          -- Array of length 2
      21       -- -2
      19 6AB3  -- 27315

Here is the actual representation:

C4             -- Tag 4
   82          -- Array of length 2
      02       -- 2
      19 6AB3  -- 27315
cowtowncoder commented 6 years ago

Ok. Sounds like there is a problem, and looks like information included should be enough to reproduce the problem as a unit test. Thank you for reporting this.

cowtowncoder commented 5 years ago

Hmmh. Ok, yes. Looks like I interpreted exponent the wrong way, but for better or worse it is consistently so, meaning that Jackson can read content it writes.

The challenge here is backwards compatibility. I think that I will fix this for 2.10, instead of 2.9 patch, since this does change working quite drastically.

wlukowicz commented 5 years ago

Looks good, thanks for fixing.

cynogit commented 3 years ago

@cowtowncoder, recently tried to migrate from 2.9.9. It was a stroke of big luck we found this breaking change you introduced before it reached our production. Did you consider adding a DeserializationOption and a default value safe to the people who already have the older format in their production DBs? Why doesn't Jackson release notes mark this commit as breaking change?

It's a great shame, guys, great shame :-(

cowtowncoder commented 3 years ago

@cynogit Mark what as breaking change? A fix? What kind of sense would that make? Or perpetuating incorrect encoding?

It is unfortunate that the bug existed of course. I will add a note on 2.10 release wiki page, but this is the first time I have heard a complaint from users.

Also: if you wanted to actually help instead of bitch and whine about a free product, you are free to help with release candidate process: this is where most compatibility problems are found, by people who are collaborating on improving things. Instead of simply enjoying free labor of others.

cynogit commented 3 years ago

@cowtowncoder, consider you already have a production DB written with 2.9. Reading it with 2.10 will produce objects with incorrect values. That is why it is a breaking change.