sfackler / rust-postgres

Native PostgreSQL driver for the Rust programming language
Apache License 2.0
3.49k stars 443 forks source link

Handling of numeric columns. #307

Open ivanceras opened 6 years ago

ivanceras commented 6 years ago

I think numeric columns should be handled in rust-postgres. Numeric columns are very common data types for high precision requirements. Looking at diesel-pg implementation, it seems to be converting the raw vec to bigdecimal and bigint.

https://github.com/diesel-rs/diesel/blob/master/diesel/src/pg/types/numeric.rs

bigdecimal num_bigint

I couldn't wrap my head around the implementation, it's more complex that I though it would be.

jwilm commented 6 years ago

I would also enjoy having this feature. In the mean time, I've been querying them as colname::text and parsing the result.

sfackler commented 6 years ago

Sure, I'd be happy to take a PR adding support via bigdecimal!

sfackler commented 6 years ago

How is NaN handled?

ivanceras commented 6 years ago

Shouldn't be Nan just be treated as Error in Result<T,Error> ?

sfackler commented 6 years ago

It could be, but it seems like you'd want to be able to at least have the ability to read a NaN out of the database. I guess it could just be a wrapper enum though.

ivanceras commented 6 years ago

@sfackler yes, the Nan types is a variant in the enum.

https://github.com/diesel-rs/diesel/blob/master/diesel/src/pg/types/floats/mod.rs#L12

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum PgNumeric {
    Positive {
        weight: i16,
        scale: u16,
        digits: Vec<i16>,
    },
    Negative {
        weight: i16,
        scale: u16,
        digits: Vec<i16>,
    },
    NaN,
}

I may have to play a bit more on these numeric types before I could make a PR.

ivanceras commented 6 years ago

I already have a working numeric handling for postgres. https://github.com/ivanceras/rustorm/blob/master/src/pg/numeric.rs I'm still figuring out how to integrate this into the rust-postgres source code.

fromheten commented 6 years ago

This is a features I'd like to have in rust-postgres.

In order to put my money where my mouth is I therefore want to set a €100 bounty on this.

I'll pay the money to whoever submits a patch (under the same licence as this project, MIT) that lets me do #[derive(FromSql, ToSql)] on a struct where one member is of type num_bigint::BigUint or equivalent (I'm just looking for positive integers in my application). If it uses a different but equivalent library than num_bigint that's all OK for me. What I'm looking for is being able to have an any-precision positive integer in my struct, and be able to treat it as a number type in PG, and for this to work as great as the rest of this library (great work @sfackler).

Whoever does that and sends me an invoice for €100 will get it! Can pay in Bitcoin Core/Bitcoin Cash, Ethereum, SEPA transaction or cash plus a beer or coffee if you're around Berlin, Germany.

I'll give it a go myself as well, but hope someone else will be faster than me. @ivanceras what you have seems like a great start.

ivanceras commented 6 years ago

I was trying to integrate numeric into rust-postgres but I couldn't get the current tests pass. https://hub.docker.com/r/sfackler/rust-postgres-test/ looks like the test image is private. @sfackler can you opensource the Dockerfile used to build the image for the rust-postgres-test setup?

kestred commented 6 years ago

I'd particularly like to vote on Numeric support for bigdecimal, with the ongoing optimisim that eventually bigdecimal will at some point become part of num again.

I'm currently using the same work around as @jwilm (mynumeric::text); but if a feature for the bigdecimal crate would be welcome, I'd be happy to put together a PR.

sfackler commented 6 years ago

@ivanceras Hmm, it seems to be public as far as I can tell - I don't think Circle would be able to pull it otherwise.

Definitely happy to support bigdecimal/whatever else!

sfackler commented 6 years ago

The docker build for the image is over here though in any case: https://github.com/sfackler/rust-postgres/tree/master/docker

kdar commented 6 years ago

rust_decimal has an implementation for rust-postgres it it helps anyone: https://github.com/paupino/rust-decimal/blob/126271f9ef9a1e599bcd55eee70657486be0530f/src/postgres.rs

rust_decimal performs pretty well except for dividing, in case anyone was curious:

test tests::bench_bigdecimal_add   ... bench:     541,332 ns/iter (+/- 124,543)
test tests::bench_bigdecimal_div   ... bench:     559,528 ns/iter (+/- 190,289)
test tests::bench_bigdecimal_mul   ... bench:     493,652 ns/iter (+/- 16,276)
test tests::bench_bigdecimal_sub   ... bench:     344,488 ns/iter (+/- 58,812)
test tests::bench_decimal_add      ... bench:     112,622 ns/iter (+/- 5,842)
test tests::bench_decimal_div      ... bench:     362,076 ns/iter (+/- 75,458)
test tests::bench_decimal_mul      ... bench:     239,117 ns/iter (+/- 31,331)
test tests::bench_decimal_sub      ... bench:     542,171 ns/iter (+/- 34,186)
test tests::bench_rust_decimal_add ... bench:     104,577 ns/iter (+/- 4,711)
test tests::bench_rust_decimal_div ... bench:   1,201,631 ns/iter (+/- 77,024)
test tests::bench_rust_decimal_mul ... bench:      55,916 ns/iter (+/- 14,682)
test tests::bench_rust_decimal_sub ... bench:      73,354 ns/iter (+/- 3,539)
fromheten commented 6 years ago

@kdar that seems to be solving the problem I had at hand. Thanks for sharing! Gonna give it a go on Monday.

jspeis commented 5 years ago

Would anyone happen to know how I could integrate the rust_decimal support to work with the master branch version of tokio-postgres?

mamcx commented 5 years ago

I also get hit with this. I manage financial data, and need support for decimal/money calculations. I tough https://docs.rs/rust_decimal/1.0.1/rust_decimal/struct.Decimal.html was preferred over bigdecimal? I already use it in my codebase so if the community is moving in to big decimal I miss it.

Also, is not possible to convert to String? I could hack stuff for now: let x:Option<String> = FromSql::from_sql(ty, raw)?; //but get mangled data

Using manual ::text conversion is not possible for me. I need to handle data in databases where I don't own the schemas and query of views/functions will become impossible this way..

godofdream commented 3 years ago

https://github.com/paupino/rust-decimal has an integration for tokio-postgres. I propose to point out this in the readme about types, and maybe close this ticket.

tthug commented 1 year ago

First of all thank you for your work on this project and contributions to these discussions. Using the rust_decimal crate seems to be the recommended approach for numerics with rust-postgres. I was wondering if any of you could help me out with figuring out how to deserialize a numeric columns into the rust decimal type.

When I write to the database I can easily create the parameter and insert it into a column tagged as a ::NUMERIC type by doing the following:

let numeric_type = Type::new("".into(), 0, Kind::Simple, "".into());
let mut bytes = BytesMut::new();
let decimal_offset = Decimal::from_u64(data.offset).unwrap();
decimal_offset.to_sql(&numeric_type, &mut bytes).unwrap();

I got this approach from the rust_decimal tests A reference to the decimal offset can be passed directly to the insert query as a part of tha params slice.

However, when I want to read a value from this column later on, I have trouble deserializing the column

if let Some(row) = rows.get(0) {
    let numeric_type = Type::new("".into(), 0, Kind::Simple, "".into());
    let bytes: &[u8] = row.get("max");
    let output = Decimal::from_sql(&numeric_type, &bytes).unwrap();
    output.to_string().as_str().parse::<u64>().unwrap()
} else {
    0
}

This is fetching the max column which is a max aggregate of the values of the offset colum typed as a NUMERIC. This compiles and seems okay but when I run the application it runs into a runtime panic with the following message:

thread 'main' panicked at 'error retrieving column max: error deserializing column 0: cannot convert between the Rust type `&[u8]` and the Postgres type `numeric`', ./external/crate_index__tokio-postgres-0.7.7/src/row.rs:151:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I am wondering what I might be doing wrong here, any help would be greatly appreciated.

For context I have the postgres, db-postgres and db-tokio-postgres features enabled for the rust_decimal create.

tthug commented 1 year ago

For anyone who runs into this in the future: the explicit casting I was doing was unnecessary.

By pulling the rust_decimal package in scope with use rust_decimal::Decimal; you get an implementation of ToSQL/FromSQL which in my case tokio-postgres automagically picks up and handles the conversion. So the column deserialization ended up being just:

if let Some(row) = rows.get(0) {
    let output: Decimal = row.get("max"); 
    output.to_string().as_str().parse::<u64>().unwrap()
} else {
    0
}
elfedy commented 1 year ago

For cases where more than 96 bits need to be supported, there's a library with a dedicated data type: https://github.com/tzConnectBerlin/rust-pg_bigdecimal