Deserializing BigDecimal using JsonNode loses precision

[TomCLForwood]

Using jackson-databind 2.9.6

There seems to be an issue parsing BigDecimals as JsonNodes where it is forced to pass though an intermediate phase as a double and precision is destroyed

The code below illustrates the issue - on the original BigDecimal the precision is 2, after deserializing the precision is 1. If the custom Deserializer is disabled then the test passes.

import java.io.IOException;
import java.math.BigDecimal;

import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;

class SimpleTest {

	@Test
	void test() throws JsonParseException, JsonMappingException, IOException {
		TestClass z = new TestClass();
		z.setA(new BigDecimal("0.0060"));
		
		ObjectMapper m = new ObjectMapper();
        m.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);
		String s = m.writeValueAsString(z);
		System.out.println(s);
		
		TestClass readValue = m.readValue(s, TestClass.class);
		System.out.println(readValue.getA());
		assertEquals(z.a.precision(), readValue.a.precision());
	}
	
	static class DeSer extends StdDeserializer<TestClass> {

		public DeSer() {
			super(TestClass.class);
		}

		@Override
		public TestClass deserialize(JsonParser jp, DeserializationContext ctxt)
				throws IOException, JsonProcessingException {
			JsonNode node = jp.getCodec().readTree(jp);
			System.out.println(node.toString());
			ObjectMapper m = new ObjectMapper();
			BigDecimal bd = m.treeToValue(node.get("a"), BigDecimal.class);
			TestClass testClass = new TestClass();
			testClass.setA(bd);
			return testClass;
		}
		
	}

	@JsonDeserialize(using = DeSer.class)
	static class TestClass {
		BigDecimal a;

		public BigDecimal getA() {
			return a;
		}

		public void setA(BigDecimal a) {
			this.a = a;
		}
		
	}
}```

[cowtowncoder]

Quick question: could this be same as #1770 ?

[weiweiracings]

I meet this problem too;

JsonNode node = jp.getCodec().readTree(jp);
this method will lose precision

[ymenager]

Yes indeed, I just noticed it because my accounting reconciliation processing was giving me unexpected results.

This bug should at least be prioritize as high since it's impact is extremely disastrous for anything where number accuracy matter (ie: financial transactions !)

[cowtowncoder]

@ymenager Please do tell me how I use my free time. I really like hearing FUCKING DEMANDS FROM YOU, A PERSON I DON'T EVEN KNOW.

Actually, fuck you. Go away.

[ymenager]

jeez no need to go on name calling, and I didn't say it had to be FIXED urgently, i said it had to be prioritized urgently.... not the same thing in the open source world (people just work on whatever they want, but they need to know which issues need to be addressed urgently or not)

right now anyway who looks at the issue's labels can't see this is actually an extremely severe issue (data corruption is the most disastrous issue a data read/write framework can have)

[CowboyCodingRules]

@cowtowncoder Do you need a hug?

[GedMarc]

Honestly it just sounds like you're using Oracle DB and float data column types - you get the same thing.
And that ladies and gentleman, is the first time that i've seen in 8 years @cowtowncoder lost it online :)

So it works like this @ymenager

Double doesn't really have a "precision", or one that you can set anyway, so it adopts the ISO standard, when porting a double into a BigDecimal (which works on scale/precision) you need to be aware of a few things

1. When going from double to big decimal you will have to round using some or other method, Round UP, round Down etc etc

• So a quick Q - how would you change the deserializer to provide the correct rounding mechanism for the conversion of double to BigDecimal, since you cannot do it without the rounding (and that obviously makes sense)

2. BigDecimal from real/double/float you will always lose cents and they are always difficult to track. This has to do with the formula's used for the number types, and the mandatory rounding mode. The base problem here is actually incorrect data types chosen.

3. Trying to order a prioritization of someone's time and effort is considered extremely rude, unless you're paying them, in which case it is called a Job and your time is actually hired. He did ask if it was a problem similar to another one. Perhaps it's all part of the same thing?
   And of course, maybe it is also a misunderstanding of moving from a linear algorithm (double) to a scaler algorithm (BigDecimal)

4. Don't bite the hand that feeds you. He wrote this library. If you want something from someone, you smile and wave (says the one with Asperger's)

So, down to the grit.

e.g.

public static void main(String[] args)
{
    int count = 0, errors = 0;
    int scale = 2;
    double factor = Math.pow(10, scale);
    MathContext mc = new MathContext(16, RoundingMode.DOWN);
    for (double x = 0.0; x < 1; x += 0.0001)
    {
        count++;
        double d = x;
        d = Math.round(d * factor) / factor;
        BigDecimal bd = new BigDecimal(d, mc);
        bd = bd.remainder(new BigDecimal("0.01"), mc);
        if (bd.multiply(BigDecimal.valueOf(100)).remainder(BigDecimal.ONE, mc).compareTo(BigDecimal.ZERO) != 0)
        {
            System.out.println(d + " " + bd);
            errors++;
        }
    }
    System.out.println(count + " trials " + errors + " errors");
}

If you run this you can see what is wrong, you cannot go directly from double to BigDecimal without specifying a rounding. They are different storage and formula based mechanisms.

Hope this helps, I have another one which goes into more detail (especially with handling BigDecimal.Zero)

[GedMarc]

@ymenager Here #1568 (comment)

It seems this misunderstanding of doubles and big decimals is quite common

[ymenager]

@GedMarc Yes I know very well what you mean in regards to double / float and precisions.

The problem is those numbers should NEVER be stored as double or floats in the first place, but always be stored in a reliable format ( BigDecimal ). There are use cases where losing precision is acceptable, a json parsing library is definitely NOT one of them.

And again, although it might have interpreted that way ( I might have not worded things the best way and for that I do apologise ), I was not asking to prioritise someone's time... I was talking about prioritizing the issue (ie... the severity label)

I know this is an open source project (I've been doing OS, so I don't expect ANYONE to actually go and fix it, however I do expect issues to properly triaged so that anyone who looks at the issues can see what issues might affect them, and then if they are interested they can go and fix it.

This is ESPECIALLY important for silent and random data corruption issue which might affect many people without them being aware of it, without potentially very bad or even catastrophic results especially for such a widely used library !!!! ( How many people use jackson for finance or health related projects ??????? )

[GedMarc]

Sigh....
You really are not a pleasant person, with a flair for end of the world scenarios.

Here.

It was not a pleasure.

[GedMarc]

@cowtowncoder Can close.

[plokhotnyuk]

@ymenager parsing of java.math.BigDecimal is a very sensitive process. You should set
reasonable limits for number of significant digits and scale to avoid DoS vulnerabilities for systems that are opened to the wild wide world: https://github.com/plokhotnyuk/jsoniter-scala/blob/33dcb87459bcb87f213acc12fa3a3359a5596d61/jsoniter-scala-core/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonReader.scala#L1388

[ymenager]

@GedMarc Although I might be highlighting the worse case scenarios, they are quite real and not "end of the world scenarios".

I've worked most of my life on mission critical system for Banks, Telecoms, Governments so I have a deep appreciation of the real world consequences of the difference between a number going in as "3.70" and coming out as "3.7" (to quote exactly what i saw happen).

Even on a smaller scale those bugs can have severe consequences. The effect of that bug could have gotten me personally in trouble with the tax office and gotten me accused of tax fraud (!) not a minor issue (luckily I'm rather paranoid and test and double-check everything manually)

Also, from what I had read in this issue and another referred, doing those two features does NOT solve the problem (unless the information I read was incorrect or i mis-read it ? I didn't actually go verify it.... I'll give it a try)

@plokhotnyuk Yes you have a very good point about potential abuse of the data being stored in BigDecimal format. Makes me think the safest way to handle this would have been for jackson to always store those as strings no matter what, and then only use "on demand" conversion... But that's probably too big a change to implement and not realistic

[GedMarc]

Enough - the problem between the keyboard and the chair.
First thing first, make sure it is a bug. It is not.

As you said, there are literally thousands of finance (including mine) and transaction heavy businesses running the software, You're the only to have this problem. Find the denominator.

It's a simple configuration of the ObjectMapper, which obviously @cowtowncoder knew, but your attitude is a really big problem.

Your test case works when in the correct config, your problem is solved, just a lack of understanding on the API, and angry emotion driven responses to everyone trying to help you.
Go throw your rants elsewhere.

[ymenager]

Oh actually after running that specific code in this ticket, i see the issue is slightly different from what I was looking at (which was that after parsing the value using jsontree it was storing internally the value as Double although that flag was enabled to use BigDec, basically before involving any kind of data binding)......

So I might have over-reacted or at the very least ranted in the wrong ticket :) I guess I'm so used to see major consequences of small bugs of that category that it's made me oversensitive on the topic, and it rubbed me wrong when i thought it was a "known issue" that seemed ignored.

I'll go relook at the issue I saw to double check

[GedMarc]

@ymenager Thank you, Much appreciated

With that description, it changes a bit.
This was a major one that I think is the core of it all

ObjectMapper m = new ObjectMapper();
m.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);

You aren't assigning the configured instance back into "m", so technically it was never actually "configured" or "used". with/without/configure, use the returned instance.
Also, try to stick on the newer methods, enable/disable, and chain if you can.

ObjectMapper m = new ObjectMapper();
		m = m.disable(USE_BIG_DECIMAL_FOR_FLOATS)
				.enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN)
				.setNodeFactory(JsonNodeFactory.withExactBigDecimals(true));

[ymenager]

@GedMarc
Actually after double-checking i found that the problem I faced was a combination of multiple issues who joined hands to make my life difficult

I did had USE_BIG_DECIMAL_FOR_FLOATS, and then when i debugged the code it was popping up as Double in the nodetree (so instant potential data corruption). Which led to me think there was a bug where when using nodetree it ignored USE_BIG_DECIMAL_FOR_FLOATS (and the content of this issue and another one I saw made me think that was a known issue)

However I was using Spring Boot, and the ObjectMapper was in a constructor class that was on a package that wasn't being picked up by the annotation scanner, so spring boot "helpfully" automatically created a default one (because it uses it for the REST), so it wasn't visible that the object mapper was not really the one I had configured :(

After checking by hardcoding the Object Mapper (rather than using injected one), I can see that it has now created a DecimalNode which is what i expected rather than using a Double.

So I do apologise for my overreaction, too much dealing with nasty data corruption bugs from "corporate developers" (the majority of which don't even seem to know that BigDec exists sigh) might have made me a bit too oversensitive on that topic

[cowtowncoder]

Ok. So, I am back & apologies for over-reacting as well. I did read more into comment than was probably intended, but I can see how it was not meant the way I read it. I have nothing against anyone explaining why issue is critical and making their case.

I see there has been in-depth discussion on actual issues on handling of fractional numbers, which is good.
About the only thing missing was the usual angle on "but in Javascript it's always IEEE 64-bit double anyway" which we can ignore here.

Now, looking back at the original description, I can see couple of potential problems due to shuffling back and forth via streaming (getCodec().parseTree(); m.treeToValue()), and possibly missing configuration.
There is also one setting not mentioned which may be relevant is

JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN

although it not being enabled probably makes this non issue.

But. Fundamentally I am not sure it is possible to fully retain precision property of BigDecimal values through conversions. If this is possible and someone can point out likely place where trimming occurs, great, I am happy to change it.

One suggestion I have, however, is to streamline custom deserializer like so:

		@Override
		public TestClass deserialize(JsonParser jp, DeserializationContext ctxt)
				throws IOException, JsonProcessingException {
//			JsonNode node = jp.getCodec().readTree(jp);
			JsonNode node = ctxt.readTree(jp)
//			ObjectMapper m = new ObjectMapper();
//			BigDecimal bd = m.treeToValue(node.get("a"), BigDecimal.class);
                         JsonNode value = node.path("a");
                         if (value.isNumber) {
                             bd = value.decimalValue();
                        } else {
                            // throw exception?
                        }
			TestClass testClass = new TestClass();
			testClass.setA(bd);
			return testClass;
		}

which would avoid couple of conversions and eliminate sources of errors.

[cowtowncoder]

@CowboyCodingRules thanks, hugs are always great but I think I am good now :)

[plokhotnyuk]

@cowtowncoder I have a hug for you from the OSS community.

Here is a code which creates BigDecimal instances from the decimal string representation much more efficiently.

For small numbers you will get ~2x speed up and for big numbers with 1000000 digits speed up will be ~50x times.

Fill free to adopt it in core of Jackson or just ask for help with a PR.

[cowtowncoder]

@plokhotnyuk that is indeed interesting! I'll add it to a long list of things I'd like to work on next (https://github.com/FasterXML/jackson-future-ideas/wiki/Jackson-Work-in-Progress)

[petrdvorak]

This was a fun-to-read thread... :)

@GedMarc Thank you for the hints on how to fix the issue!

[cowtowncoder]

@plokhotnyuk Created FasterXML/jackson-core#577 as placeholder, hoping someone had time and interest to look into merging optimizatons.

[pain64]

@cowtowncoder, is any workaround for version 2.10.1?

[cowtowncoder]

@over64 Workaround for what exactly? As @GedMarc points out, use

ObjectMapper m = new ObjectMapper();
m.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);

if you simply want BigDecimal to be used (to retain precision) in tree model (JsonNode). Otherwise it all depends on target Java type.

[cowtowncoder]

Closing this as general issue: does not mean there could not be room for improvement but it would need to be something that:

1. Is not configurable to work (as above, to retain BigDecimal for tree model
2. Could be improved without breaking existing or adding overhead for use case that do not benefit from changes.

[pain64]

@cowtowncoder, yes, it parsed as big decimal, but result is so weird?

   @Test
    void omTest() throws IOException {
        var mapper = new ObjectMapper();
        mapper.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);
        var x = mapper.readTree("10.00"); // yields BigDecimal(1E-1) ???
        x.decimalValue().scale(); // -1 but expected 2
        mapper.writeValueAsString(x); // "1+E1" ???
    }

    @Test
    void gsonTest() throws IOException {
        var x = new Gson().toJsonTree("10.00");
        x.getAsBigDecimal().scale(); // 2
    }

[petrdvorak]

@over64

mapper = mapper.setNodeFactory(JsonNodeFactory.withExactBigDecimals(true));

[pain64]

@petrdvorak, thanks a lot!

[prakashtanaji]

Having the below two lines(together) resolved my issue.

objectMapper.setNodeFactory(JsonNodeFactory.withExactBigDecimals(true)); objectMapper.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);

[harsha-sira]

This thread saved my day !

[akamuza]

What a helpful old thread!!