paupino / rust-decimal

Decimal number implementation written in pure Rust suitable for financial and fixed-precision calculations.
https://docs.rs/rust_decimal/
MIT License
972 stars 177 forks source link

Bug: Decimal::from_sql incorrectly transforms Postgres NaN to 0 #655

Closed lukoktonos closed 5 months ago

lukoktonos commented 5 months ago

Postgres Numerics have 3 special values: NaN, Infinity, and -Infinity.

The FromSql implementation of Decimal currently returns an error when parsing Infinity and -Infinity because they have scales beyond what the crate accepts--this is fine, although it may be better to explicitly reject these values rather than relying on the scale, if they are not representable in Decimal.

But for NaN, from_sql silently converts it into a 0 because it is only checking if the sign is negative:

            neg: sign == 0x4000,

This is what the raw bytes of NaN look like in from_sql:

[/Users/luke/code/rust-decimal/src/postgres/driver.rs:68] &raw = Cursor {
    inner: [
        0,
        0,
        0,
        0,
        192,
        0,
        0,
        0,
    ],
    pos: 4,
}
[/Users/luke/code/rust-decimal/src/postgres/driver.rs:70] &sign = 49152

The relevant masks from postgres are:

#define NUMERIC_SIGN_MASK   0xC000
#define NUMERIC_POS     0x0000
#define NUMERIC_NEG     0x4000
#define NUMERIC_SHORT       0x8000
#define NUMERIC_SPECIAL     0xC000
#define NUMERIC_NAN     0xC000
#define NUMERIC_PINF        0xD000
#define NUMERIC_NINF        0xF000

My proposed logic for handling this is something like:

if sign & NUMERIC_SIGN_MASK) == NUMERIC_SPECIAL {
  if sign == NUMERIC_NAN || sign == NUMERIC_PINF || sign == NUMERIC_NINF {
     return some sort of Unsupported Error
  }
}
... continue parsing

If that logic sounds good I'd be happy to submit a patch

paupino commented 5 months ago

Great callout and a very well written up issue!

I agree with the reasoning regarding capturing and throwing an error. I'd be more than happy to review any patches submitted and really appreciate the offer to help - thank you!