Diesel: Diesel got wrong value but no error message

[ciyool]

Versions

• Rust:
  1.36
• Diesel:
  1.4.2
• Database:
  PostgreSQL 10.0.3
• Operating System
  Windows Server 2012 R2 Datacenter

Feature Flags

• diesel:

Problem Description

If i set pg table column type float8，and diesel schema table! column type Float. I can't get true value stored in db, and did not display any error messages.
example:
db column value 5.53，but diesel show 2.345625

What is the expected output?

db column value 5.53 and diesel show 5.53
or show “type not match” error message

What is the actual output?

show 2.345625 with no error message

Are you seeing any additional errors?

No

Steps to reproduce

sql:
CREATE TABLE "public"."table1" (
  "test_column" float8
);
INSERT INTO "public"."table1"("test_column") VALUES (5.53) 

schema.rs
table! {
    table1 (id) {
        test_column -> Float,
    }
} 
model.rs
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Queryable)]
pub struct TestModel {
    pub test_column : f32,
}

service.rs
let mut s_row: Vec<TestModel> = table1::table
            .load(conn)?;

[weiznich]

FLOAT8 matches diesel::sql_types::Double, diesel::sql_types::Float matches Float4. (Or just use Float4 and Float8 as sql type in your table definition or use diesel print-schema which does this automatically)

[ciyool]

I know how to fix this error. I hope that when I specify a type error, diesel can give an error instead of displaying an incorrect result. The error message can help me find bugs in time.

[weiznich]

There is no way for us to know what's the correct type in general case, so this is impossible to do.

[ciyool]

I tried:

rust code:
    let foo:f64=5.53;
    println!("foo: {}",foo);
    let bar:f32=foo as f32;
    println!("bar: {}",bar);

result:
  foo: 5.53
  bar: 5.53

why diesel return 2.345625 ?

[weiznich]

Casting a between f32 and f64 is something entirely different to parsing postgres binary protocol.

[sgrif]

We have debug assertions on Integer and BigInteger for receiving too few bytes. We only have assertions on FromSql<Double, DB> for f64 for too many bytes. The same error you get from miscasting int4 to int8 should probably be given if you miscast float4 to float8

[tommket]

Hi,
This is my first Diesel issue and this issue has a "good first issue" label, so I have some additional questions.

I looked up how it is done for Integer in mysql in https://github.com/diesel-rs/diesel/blob/7e676025dd11fa5b50399a2dcf9887b37280abe6/diesel/src/mysql/types/primitives.rs

Something like:

impl FromSql<Integer, Mysql> for i32 {
    fn from_sql(value: MysqlValue<'_>) -> deserialize::Result<Self> {
        use crate::mysql::NumericRepresentation::*;

        match value.numeric_value()? {
            Tiny(x) => Ok(x.into()),
            Small(x) => Ok(x.into()),
            Medium(x) => Ok(x),
            Big(x) => x.try_into().map_err(|_| {
                Box::new(DeserializationError(
                    "Numeric overflow/underflow occured".into(),
                )) as _
            }),
            Float(x) => f32_to_i64(x).and_then(|i| {
                i.try_into().map_err(|_| {
                    Box::new(DeserializationError(
                        "Numeric overflow/underflow occured".into(),
                    )) as _
                })
            }),
            Double(x) => f64_to_i64(x).and_then(|i| {
                i.try_into().map_err(|_| {
                    Box::new(DeserializationError(
                        "Numeric overflow/underflow occured".into(),
                    )) as _
                })
            }),
            Decimal(bytes) => decimal_to_integer(bytes),
        }
    }
}

Then I looked up how it is done for Double in mysql:

impl FromSql<Double, Mysql> for f64 {
    fn from_sql(value: MysqlValue<'_>) -> deserialize::Result<Self> {
        use crate::mysql::NumericRepresentation::*;

        match value.numeric_value()? {
            Tiny(x) => Ok(x.into()),
            Small(x) => Ok(x.into()),
            Medium(x) => Ok(x.into()),
            Big(x) => Ok(x as Self),
            Float(x) => Ok(x.into()),
            Double(x) => Ok(x),
            Decimal(bytes) => Ok(str::from_utf8(bytes)?.parse()?),
        }
    }
}

1. I just cannot find similar implementations of the FromSql trait for postgres. The primitives.rs contains an implementation only for bool: https://github.com/diesel-rs/diesel/blob/7e676025dd11fa5b50399a2dcf9887b37280abe6/diesel/src/pg/types/primitives.rs
2. I guess that also the impl FromSql<Double, Mysql> for f64 will have to be modified to properly create a DeserializationError for Float(x), or is the problem described in this issue specific to only postgres?

[weiznich]

The corresponding impls for postgresql are here.

Solving this issue is much simpler than what is done for mysql. We just want to add additional debug_assert! there that we've got the correct number of bytes. For the linked impl there is already one for getting to few bytes, we just want another one that checks if we got more than 4 bytes which panics with an error message saying that the user likely tried to convert a Double column to Float. Basically like it's done here for SmallInt

[tommket]

We just want to add additional debug_assert! there that we've got the correct number of bytes. For the linked impl there is already one for getting to few bytes, we just want another one that checks if we got more than 4 bytes which panics with an error message saying that the user likely tried to convert a Double column to Float.

The current implementation for Float seems to already be checking exactly that. I also got initially confused by the text in the debug_assert saying the opposite of what the logical expression has.

impl<DB> FromSql<sql_types::Float, DB> for f32
where
    DB: Backend + for<'a> BinaryRawValue<'a>,
{
    fn from_sql(value: crate::backend::RawValue<DB>) -> deserialize::Result<Self> {
        let mut bytes = DB::as_bytes(value);
        debug_assert!(
            bytes.len() <= 4,
            "Received more than 4 bytes while decoding \
             an f32. Was a double accidentally marked as float?"
        );
        bytes
            .read_f32::<DB::ByteOrder>()
            .map_err(|e| Box::new(e) as Box<dyn Error + Send + Sync>)
    }
}

[weiznich]

Can you verify that the assertion is actually triggered by the example provided above? If that's the case the issue is already fixed. If not we need to investigate a bit more why the assert is not triggered in this case.

[tommket]

Yes, I am going to try and reproduce the issue with:

Versions

• Rust:
  1.51
• Diesel:
  1.4.6
• Database:
  PostgreSQL 13.2
• Operating System
  Ubuntu 20.04

[tommket]

So did a little local test and Diesel panicks with:

...
panicked at 'Received more than 4 bytes while decoding an f32. Was a double accidentally marked as float?', /***/.cargo/registry/src/github.com-1ecc6299db9ec823/diesel-1.4.6/src/type_impls/floats.rs:13:9
...

[weiznich]

Perfect, thanks for validating this.
That means this issue is already fixed :tada: