/diesel

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

Attempting to serialize `BigDecimal::from_str("10000.16")` fails on PG #1044

Closed
sgrif opened this Issue Jul 26, 2017 · 5 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@sgrif @Eijebong @lancecarlson
@sgrif
Member

sgrif commented Jul 26, 2017

To reproduce:

let connection = PgConnection::establish("...").unwrap();
let value = BigDecimal::from_str("10000.16");
sql::<Numeric>("SELECT $1")
    .bind::<Numeric, _>(value)
    .execute(&connection)
    .unwrap()
@sgrif

This comment has been minimized.

Sign in to view
Member

sgrif commented Jul 26, 2017

/cc @rubdos
@Eijebong

This comment has been minimized.

Sign in to view
Member

Eijebong commented Jul 26, 2017

10 000 is suspicious since pg stores numeric values in base 10k
@lancecarlson

This comment has been minimized.

Sign in to view
Contributor

lancecarlson commented Jul 27, 2017

When I encountered this issue, the error I got was:

error importing: DatabaseError("invalid digit in external "numeric" value")
@sgrif

This comment has been minimized.

Sign in to view
Member

sgrif commented Jul 27, 2017

The representation I get for SELECT '10000.16'::numeric is Positive { weight: 1, scale: 2, digits: [1, 0, 1600] }. The value we're sending is Positive { weight: 0, scale: 2, digits: [10000, 1600] }
@sgrif

This comment has been minimized.

Sign in to view
Member

sgrif commented Jul 27, 2017

The fix is relatively trivial. Just improving our test coverage and making sure this doesn't affect numbers after the decimal point as well

sgrif added a commit that referenced this issue Jul 27, 2017

@sgrif
Properly encode `BigDecimal` values whose first digit is 10k 
When serializing a numeric column, we need to take a decimal value and
convert it to a list of its digits in base 10k. The serialization code
had a bug when the first digit was exactly `10000`, resulting in the
digits being `[10000, ...]` instead of `[1, 0, ...]`. This bug only
affected the integral part, as the decimal portion uses different logic
in order ot strip trailing zeroes.

Fixes #1044.
Loading status checks…
fb1ca8d

@sgrif sgrif referenced this issue Jul 27, 2017

Merged

Properly encode `BigDecimal` values whose first digit is 10k #1046

sgrif added a commit that referenced this issue Jul 27, 2017

@sgrif
Properly encode `BigDecimal` values whose first digit is 10k 
When serializing a numeric column, we need to take a decimal value and
convert it to a list of its digits in base 10k. The serialization code
had a bug when the first digit was exactly `10000`, resulting in the
digits being `[10000, ...]` instead of `[1, 0, ...]`. This bug only
affected the integral part, as the decimal portion uses different logic
in order ot strip trailing zeroes.

Fixes #1044.
Loading status checks…
e1bbdf7

@sgrif sgrif closed this in #1046 Jul 27, 2017

Fiedzia added a commit to Fiedzia/diesel that referenced this issue Jul 31, 2017

@sgrif @Fiedzia
Properly encode `BigDecimal` values whose first digit is 10k 
When serializing a numeric column, we need to take a decimal value and
convert it to a list of its digits in base 10k. The serialization code
had a bug when the first digit was exactly `10000`, resulting in the
digits being `[10000, ...]` instead of `[1, 0, ...]`. This bug only
affected the integral part, as the decimal portion uses different logic
in order ot strip trailing zeroes.

Fixes diesel-rs#1044.
5f73359
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment