Sign loss while parsing rust_decimal::Decimal to sqlx::postgres::types::PgMoney

[Kengyen]

    let foo = Decimal::new(-314, 2);
    let bar = PgMoney::from_decimal(foo, 2);
    let baz = bar.to_decimal(2);
    println!("foo={} bar={:?} baz={}", foo, bar, baz); // foo=-3.14 bar=PgMoney(314) baz=3.14

[abonander]

This is likely due to PgMoney::from_decimal() taking bytes 4..12 of Decimal::serialize() and just treating that as a little-endian twos-complement integer:

sqlx/sqlx-core/src/postgres/types/money.rs

Lines 59 to 62 in be189bd

	let mut buf: [u8; 8] = [0; 8];
	buf.copy_from_slice(&cents.serialize()[4..12]);
	Self(i64::from_le_bytes(buf))

However the sign is the MSB of byte 3, so the value isn't actually twos-complement; we also shouldn't be ignoring scale which an integer in bytes 1..2.

It actually doesn't make much sense that scale being passed in here, I think that's supposed to be the precision that the Postgres database is expecting.

The docs make it sound like SQLx is just going to figure that out for you, but that's honestly way out of scope here because that would mean:

• Executing SHOW lc_monetary; on the database server
• Figure out what frac_digits is in that locale (assuming the client and server even have the same definition)
  • Use libc::set_locale() and localeconv()->frac_digits?
    • not thread-safe
    • changes the output of a bunch of C functions across the entire process
    • sometimes is set to nonsense anyway
  • Shell out to locale <locale name> frac_digits and read the response?
    • non-portable
  • Just assume the locale on the client is the same as on the server and use localeconv()->frac_digits?
    • error-prone
    • still highly unsafe

It would be easier to execute select 0::money::text and then count the number of zeroes before a non-digit character (which would cover locales that use a different decimal separator than .) but what about locales that don't use Arabic numerals or use right-to-left writing? It'd be a nightmare to cover everything and this is all still assuming we're talking to the database and patching the value up on-the-fly.

So yeah, instead we should make it clear that scale should be the precision that the Postgres server is expecting or else it'll interpret the value incorrectly.

I defended money in a previous issue but now that I'm digging more into it, it's honestly super gross. To be fair, a lot of that is also the POSIX locale system which is what, a 20 year old API?