Clarification on `postgres_ty_for_value` weird type values

[RoyalFoxy]

Looking through the Postgres driver I saw that the postgres_ty_for_value returns the Postgres type for a specific value type.

Taking a closer look however at the number types makes me a bit confused.

fn postgres_ty_for_value(value: &stmt::Value) -> Type {
    match value {
        // ...
        stmt::Value::I8(_) => Type::INT2,
        stmt::Value::I16(_) => Type::INT2,
        stmt::Value::I32(_) => Type::INT4,
        stmt::Value::I64(_) => Type::INT8,
        stmt::Value::U8(_) => Type::INT2,
        stmt::Value::U16(_) => Type::INT4, // <- Shouldn't this be Type::INT2 ?
        stmt::Value::U32(_) => Type::INT8, // <- Shouldn't this be Type::INT4 ?
        stmt::Value::U64(_) => Type::INT8,
        // ...
        _ => todo!("postgres_ty_for_value: {value:#?}"),
    }
}

I think this just slipped through the quality control of the quite massive change of #159 which got added to main with this commit e30a79a

[RoyalFoxy]

I just checked https://github.com/tokio-rs/toasty/blob/main/crates/toasty-driver-postgresql/src/value.rs and realized that it is in fact correct this way as they get cast to their signed counterparts and there isn't an INT16 in Postgres for the U64.

I think a better way to store U16, U32 and U64 in postgres would be to convert them to their same sized signed counterparts with cast_signed() and then converting them back with cast_unsigned().

[pikaju]

@RoyalFoxy thanks for bringing this up. I agree with you that having to eat the cost of an 8 byte integer when all you really wanted was a u32 is not that great. cast_signed() could work, although I'm worried about the fact that the data would basically be "wrong" in the database until the Toasty engine reinterprets it again. So far we've taken a more conservative approach with our types and let the user opt in to more optimized versions. It might be worth making this configurable via the #[column] attribute, letting the user choose between a bigger integer, a cast_signed integer, or panicking if the u32 is too large to fit into an INT4 without wrapping. What do you think?

[RoyalFoxy]

@pikaju Thanks for the quick response! I didn't really think about the fact that the data would be wrong which ye doesn't really sound good as it would be very specific to Toasty and you'd need some form of migration script... I like the idea of having some form of toggle where the user could specify themselves if they are fine with more data usage, possibly inaccurate data or just panicking (maybe returning a result instead but thats an implementation detail I guess)

I would raise my own hand as someone who would try to implement this but sadly Im quite occupied for the next couple of weeks because of the holidays.

Ill look at it after the holidays if nothing has happened until then.