Fix handling of numeric types for mysql

weiznich · weiznich · commit be30a0feab56 · 2021-01-20T10:23:03.000+01:00
Newer libmariadb versions do perform not that much conversion logic on
requested binds, so does for example `select 1/2` result in `0.5`. As
result of a prior change we already have most of the logic to do the
correct conversion in diesel in place.
As part of this change we request change now the requested type to the
actual field type as returned by libmysqlclient/libmariadb and do the
conversion ourself as long as we are sure that the types are compatible.
Additionally the conversion code now handles cases where we receive a
number that would not be representable using the target type. For such
cases a `Numeric overflow/underflow` error is returned
diff --git a/diesel/src/mysql/connection/bind.rs b/diesel/src/mysql/connection/bind.rs
@@ -3,6 +3,7 @@ use std::mem;
 use std::ops::Index;
 use std::os::raw as libc;
 
+use super::stmt::MysqlFieldMetadata;
 use super::stmt::Statement;
 use crate::mysql::connection::stmt::StatementMetadata;
 use crate::mysql::types::MYSQL_TIME;
@@ -31,13 +32,7 @@ impl Binds {
             .fields()
             .iter()
             .zip(types.into_iter().chain(std::iter::repeat(None)))
-            .map(|(field, tpe)| {
-                if let Some(tpe) = tpe {
-                    BindData::for_output(tpe.into())
-                } else {
-                    BindData::for_output((field.field_type(), field.flags()))
-                }
-            })
+            .map(|(field, tpe)| BindData::for_output(tpe, field))
             .collect();
 
         Binds { data }
@@ -152,7 +147,156 @@ impl BindData {
         }
     }
 
-    fn for_output((tpe, flags): (ffi::enum_field_types, Flags)) -> Self {
+    fn for_output(tpe: Option<MysqlType>, metadata: &MysqlFieldMetadata) -> Self {
+        let (tpe, flags) = if let Some(tpe) = tpe {
+            match (tpe, metadata.field_type()) {
+                // Those are types where we handle the conversion in diesel itself
+                // and do not relay on libmysqlclient
+                (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::Tiny, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::UnsignedTiny, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::Short, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::UnsignedShort, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::Long, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::UnsignedLong, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::LongLong, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::UnsignedLongLong, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::Float, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_DECIMAL)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_TINY)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_SHORT)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_LONG)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_FLOAT)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_DOUBLE)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_INT24)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_NEWDECIMAL)
+                | (MysqlType::Numeric, ffi::enum_field_types::MYSQL_TYPE_LONGLONG)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_JSON)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_ENUM)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_SET)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_TINY_BLOB)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_MEDIUM_BLOB)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_LONG_BLOB)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_BLOB)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_VAR_STRING)
+                | (MysqlType::String, ffi::enum_field_types::MYSQL_TYPE_STRING)
+                | (MysqlType::Blob, ffi::enum_field_types::MYSQL_TYPE_TINY_BLOB)
+                | (MysqlType::Blob, ffi::enum_field_types::MYSQL_TYPE_MEDIUM_BLOB)
+                | (MysqlType::Blob, ffi::enum_field_types::MYSQL_TYPE_LONG_BLOB)
+                | (MysqlType::Blob, ffi::enum_field_types::MYSQL_TYPE_BLOB)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_ENUM)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_SET)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_TINY_BLOB)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_MEDIUM_BLOB)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_LONG_BLOB)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_BLOB)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_VAR_STRING)
+                | (MysqlType::Set, ffi::enum_field_types::MYSQL_TYPE_STRING)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_ENUM)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_SET)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_TINY_BLOB)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_MEDIUM_BLOB)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_LONG_BLOB)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_BLOB)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_VAR_STRING)
+                | (MysqlType::Enum, ffi::enum_field_types::MYSQL_TYPE_STRING) => {
+                    (metadata.field_type(), metadata.flags())
+                }
+
+                (tpe, _) => tpe.into(),
+            }
+        } else {
+            (metadata.field_type(), metadata.flags())
+        };
+
+        let bytes = known_buffer_size_for_ffi_type(tpe)
+            .map(|len| vec![0; len])
+            .unwrap_or_default();
+        let length = bytes.len() as libc::c_ulong;
+
+        BindData {
+            tpe,
+            bytes,
+            length,
+            is_null: 0,
+            is_truncated: Some(0),
+            flags,
+        }
+    }
+
+    #[cfg(test)]
+    fn for_test_output((tpe, flags): (ffi::enum_field_types, Flags)) -> Self {
         let bytes = known_buffer_size_for_ffi_type(tpe)
             .map(|len| vec![0; len])
             .unwrap_or_default();
@@ -937,7 +1081,7 @@ mod tests {
     ) -> BindData {
         let mut stmt: Statement = conn.raw_connection.prepare(query).unwrap();
 
-        let bind = BindData::for_output(bind_tpe.into());
+        let bind = BindData::for_test_output(bind_tpe.into());
 
         let mut binds = Binds { data: vec![bind] };
 
diff --git a/diesel/src/mysql/connection/stmt/mod.rs b/diesel/src/mysql/connection/stmt/mod.rs
@@ -12,7 +12,7 @@ use super::bind::{BindData, Binds};
 use crate::mysql::MysqlType;
 use crate::result::{DatabaseErrorKind, QueryResult};
 
-pub use self::metadata::StatementMetadata;
+pub use self::metadata::{MysqlFieldMetadata, StatementMetadata};
 
 pub struct Statement {
     stmt: NonNull<ffi::MYSQL_STMT>,
diff --git a/diesel/src/mysql/types/mod.rs b/diesel/src/mysql/types/mod.rs
@@ -109,7 +109,7 @@ impl ToSql<Unsigned<SmallInt>, Mysql> for u16 {
 
 impl FromSql<Unsigned<SmallInt>, Mysql> for u16 {
     fn from_sql(bytes: MysqlValue<'_>) -> deserialize::Result<Self> {
-        let signed: i16 = FromSql::<SmallInt, Mysql>::from_sql(bytes)?;
+        let signed: i32 = FromSql::<Integer, Mysql>::from_sql(bytes)?;
         Ok(signed as u16)
     }
 }
@@ -122,7 +122,7 @@ impl ToSql<Unsigned<Integer>, Mysql> for u32 {
 
 impl FromSql<Unsigned<Integer>, Mysql> for u32 {
     fn from_sql(bytes: MysqlValue<'_>) -> deserialize::Result<Self> {
-        let signed: i32 = FromSql::<Integer, Mysql>::from_sql(bytes)?;
+        let signed: i64 = FromSql::<BigInt, Mysql>::from_sql(bytes)?;
         Ok(signed as u32)
     }
 }
diff --git a/diesel/src/mysql/types/primitives.rs b/diesel/src/mysql/types/primitives.rs
@@ -1,9 +1,10 @@
-use std::error::Error;
-use std::str::{self, FromStr};
-
 use crate::deserialize::{self, FromSql};
 use crate::mysql::{Mysql, MysqlValue};
+use crate::result::Error::DeserializationError;
 use crate::sql_types::{BigInt, Binary, Double, Float, Integer, SmallInt, Text};
+use std::convert::TryInto;
+use std::error::Error;
+use std::str::{self, FromStr};
 
 fn decimal_to_integer<T>(bytes: &[u8]) -> deserialize::Result<T>
 where
@@ -13,32 +14,65 @@ where
     let string = str::from_utf8(bytes)?;
     let mut splited = string.split('.');
     let integer_portion = splited.next().unwrap_or_default();
-    let decimal_portion = splited.next().unwrap_or_default();
+    let _decimal_portion = splited.next().unwrap_or_default();
     if splited.next().is_some() {
         Err(format!("Invalid decimal format: {:?}", string).into())
-    } else if decimal_portion.chars().any(|c| c != '0') {
-        Err(format!(
-            "Tried to convert a decimal to an integer that contained /
-             a non null decimal portion: {:?}",
-            string
-        )
-        .into())
     } else {
         Ok(integer_portion.parse()?)
     }
 }
 
+fn f32_to_i64(f: f32) -> deserialize::Result<i64> {
+    use std::i64;
+
+    if f <= i64::MAX as f32 && f >= i64::MIN as f32 {
+        Ok(f.trunc() as i64)
+    } else {
+        Err(Box::new(DeserializationError(
+            "Numeric overflow/underflow occurred".into(),
+        )) as _)
+    }
+}
+
+fn f64_to_i64(f: f64) -> deserialize::Result<i64> {
+    use std::i64;
+
+    if f <= i64::MAX as f64 && f >= i64::MIN as f64 {
+        Ok(f.trunc() as i64)
+    } else {
+        Err(Box::new(DeserializationError(
+            "Numeric overflow/underflow occurred".into(),
+        )) as _)
+    }
+}
+
 impl FromSql<SmallInt, Mysql> for i16 {
     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),
-            Medium(x) => Ok(x as Self),
-            Big(x) => Ok(x as Self),
-            Float(x) => Ok(x as Self),
-            Double(x) => Ok(x as Self),
+            Medium(x) => x.try_into().map_err(|_| {
+                Box::new(DeserializationError(
+                    "Numeric overflow/underflow occurred".into(),
+                )) as _
+            }),
+            Big(x) => x.try_into().map_err(|_| {
+                Box::new(DeserializationError(
+                    "Numeric overflow/underflow occured".into(),
+                )) as _
+            }),
+            Float(x) => f32_to_i64(x)?.try_into().map_err(|_| {
+                Box::new(DeserializationError(
+                    "Numeric overflow/underflow occured".into(),
+                )) as _
+            }),
+            Double(x) => f64_to_i64(x)?.try_into().map_err(|_| {
+                Box::new(DeserializationError(
+                    "Numeric overflow/underflow occured".into(),
+                )) as _
+            }),
             Decimal(bytes) => decimal_to_integer(bytes),
         }
     }
@@ -52,9 +86,25 @@ impl FromSql<Integer, Mysql> for i32 {
             Tiny(x) => Ok(x.into()),
             Small(x) => Ok(x.into()),
             Medium(x) => Ok(x),
-            Big(x) => Ok(x as Self),
-            Float(x) => Ok(x as Self),
-            Double(x) => Ok(x as Self),
+            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),
         }
     }
@@ -69,8 +119,8 @@ impl FromSql<BigInt, Mysql> for i64 {
             Small(x) => Ok(x.into()),
             Medium(x) => Ok(x.into()),
             Big(x) => Ok(x),
-            Float(x) => Ok(x as Self),
-            Double(x) => Ok(x as Self),
+            Float(x) => f32_to_i64(x),
+            Double(x) => f64_to_i64(x),
             Decimal(bytes) => decimal_to_integer(bytes),
         }
     }
diff --git a/diesel_tests/tests/boxed_queries.rs b/diesel_tests/tests/boxed_queries.rs
@@ -33,7 +33,8 @@ fn boxed_queries_can_differ_conditionally() {
         All,
         Ordered,
         One,
-    };
+    }
+
     let source = |query| match query {
         Query::All => users::table.into_boxed(),
         Query::Ordered => users::table.order(users::name.desc()).into_boxed(),

Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,7 @@ impl ToSql<Unsigned<SmallInt>, Mysql> for u16 {`
`109`	`109`
`110`	`110`	`impl FromSql<Unsigned<SmallInt>, Mysql> for u16 {`
`111`	`111`	`fn from_sql(bytes: MysqlValue<'_>) -> deserialize::Result<Self> {`
`112`		`- let signed: i16 = FromSql::<SmallInt, Mysql>::from_sql(bytes)?;`
	`112`	`+ let signed: i32 = FromSql::<Integer, Mysql>::from_sql(bytes)?;`
`113`	`113`	`Ok(signed as u16)`
`114`	`114`	`}`
`115`	`115`	`}`
`@@ -122,7 +122,7 @@ impl ToSql<Unsigned<Integer>, Mysql> for u32 {`
`122`	`122`
`123`	`123`	`impl FromSql<Unsigned<Integer>, Mysql> for u32 {`
`124`	`124`	`fn from_sql(bytes: MysqlValue<'_>) -> deserialize::Result<Self> {`
`125`		`- let signed: i32 = FromSql::<Integer, Mysql>::from_sql(bytes)?;`
	`125`	`+ let signed: i64 = FromSql::<BigInt, Mysql>::from_sql(bytes)?;`
`126`	`126`	`Ok(signed as u32)`
`127`	`127`	`}`
`128`	`128`	`}`