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 conversion to PgNumeric #241

Closed
Rexagon opened this issue May 13, 2020 · 6 comments · Fixed by #243
Closed

Strange conversion to PgNumeric #241

Rexagon opened this issue May 13, 2020 · 6 comments · Fixed by #243

Comments

@Rexagon
Copy link

Rexagon commented May 13, 2020

Recently I came across strange behavior.

This code:

let value = Decimal::from_str("0.000001660").unwrap();
println!("{:?}", value);
let pg = PgNumeric::from(value);
println!("{:?}", pg);

let dec = Decimal::try_from(pg).unwrap();
println!("{:?}", dec);

prints:

0.000001660
Positive { weight: -3, scale: 9, digits: [166, 0] }
0.000000166

Trailing zero causes this problem. Value 0.00000166 is serialized how it should be

@serejkaaa512
Copy link

Seems like there is a need of normalisation before serialising decimal to Postgres value.
If I add .normalize() before to_postgres() this issue get fixed.

@paupino
Copy link
Owner

paupino commented May 13, 2020

This does look like incorrect behavior. I believe the issue is due to the weight being -3 instead of -4 so probably stems from here: https://github.com/paupino/rust-decimal/blob/master/src/postgres.rs#L237
(Edit: actually, my math isn't right... let me revisit when I can get some focus time).

While normalization works around this issue, it still means that 1 significant figure of precision is lost. I'll take a look at this later today and see what I can find.

@serejkaaa512
Copy link

Took a look again. Normalization doesn't work correctly also. For example 0.00000166650000 doesn't serialise and deserialise correctly. Comparing implementation with bigdecimal and tests in it, looks like both serialisation and deserialisation in some cases gives wrong values in postgresql.

@paupino
Copy link
Owner

paupino commented May 14, 2020

From investigation, this looks specific to the write logic as opposed to the read logic. This is definitely isolated to the to_postgres function and exists in both the Diesel logic as well as the standard PG logic. It's not trailing zeros per se, but rather trailing zeros when they extend the PG group logic (i.e. when the last group is zero).

It seems like this is to do with the algorithm doing an "exhaustive" loop until the mantissa is zero. Because it exits "early" it effectively truncates instead of adding in the trailing zero's.

I want to ensure any solution does some exhaustive testing around this scenario (e.g. including multiple trailing zero groups) so will work on the solution over the next couple of days to ensure I cover all edge cases.

@serejkaaa512
Copy link

I have made my implementation of serialise, deserialize based on PG docs and bigdecimal impls. Added some tests of trailing zeroes. If you want I can do a pr?

@serejkaaa512
Copy link

Added PR #243

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

Successfully merging a pull request may close this issue.

3 participants