Merge pull request diesel-rs#652 from diesel-rs/sg-mysql-conversions

Have MySQL convert values to the expected type when possible

Author: sgrif

diesel/src/macros/queryable.rs

@@ -166,65 +166,56 @@ mod tests {
     use expression::dsl::sql;
     use prelude::*;
     use test_helpers::connection;
-
-    cfg_if! {
-        if #[cfg(feature = "mysql")] {
-            type IntLiteralSql = ::types::BigInt;
-            type IntLiteralRust = i64;
-        } else {
-            type IntLiteralSql = ::types::Integer;
-            type IntLiteralRust = i32;
-        }
-    }
+    use types::Integer;
 
     #[test]
     fn named_struct_definition() {
         #[derive(Debug, Clone, Copy, PartialEq, Eq)]
         struct MyStruct {
-            foo: IntLiteralRust,
-            bar: IntLiteralRust,
+            foo: i32,
+            bar: i32,
         }
 
         impl_Queryable! {
             #[derive(Debug, Clone, Copy, PartialEq, Eq)]
             struct MyStruct {
-                foo: IntLiteralRust,
-                bar: IntLiteralRust,
+                foo: i32,
+                bar: i32,
             }
         }
 
         let conn = connection();
-        let data = ::select(sql::<(IntLiteralSql, IntLiteralSql)>("1, 2")).get_result(&conn);
+        let data = ::select(sql::<(Integer, Integer)>("1, 2")).get_result(&conn);
         assert_eq!(Ok(MyStruct { foo: 1, bar: 2 }), data);
     }
 
     #[test]
     fn tuple_struct() {
         #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-        struct MyStruct(IntLiteralRust, IntLiteralRust);
+        struct MyStruct(i32, i32);
 
         impl_Queryable! {
             #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-            struct MyStruct(#[column_name(foo)] IntLiteralRust, #[column_name(bar)] IntLiteralRust);
+            struct MyStruct(#[column_name(foo)] i32, #[column_name(bar)] i32);
         }
 
         let conn = connection();
-        let data = ::select(sql::<(IntLiteralSql, IntLiteralSql)>("1, 2")).get_result(&conn);
+        let data = ::select(sql::<(Integer, Integer)>("1, 2")).get_result(&conn);
         assert_eq!(Ok(MyStruct(1, 2)), data);
     }
 
     #[test]
     fn tuple_struct_without_column_name_annotations() {
         #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-        struct MyStruct(IntLiteralRust, IntLiteralRust);
+        struct MyStruct(i32, i32);
 
         impl_Queryable! {
             #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-            struct MyStruct(IntLiteralRust, IntLiteralRust);
+            struct MyStruct(i32, i32);
         }
 
         let conn = connection();
-        let data = ::select(sql::<(IntLiteralSql, IntLiteralSql)>("1, 2")).get_result(&conn);
+        let data = ::select(sql::<(Integer, Integer)>("1, 2")).get_result(&conn);
         assert_eq!(Ok(MyStruct(1, 2)), data);
     }
 }

diesel/src/mysql/connection/bind.rs

@@ -22,10 +22,11 @@ impl Binds {
         }
     }
 
-    pub fn from_output_types<Iter>(types: Iter) -> Self where
-        Iter: IntoIterator<Item=ffi::enum_field_types>,
-    {
-        let data = types.into_iter().map(BindData::for_output).collect();
+    pub fn from_output_types(types: Vec<MysqlType>) -> Self {
+        let data = types.into_iter()
+            .map(mysql_type_to_ffi_type)
+            .map(BindData::for_output)
+            .collect();
 
         Binds {
             data: data,
@@ -43,6 +44,7 @@ impl Binds {
 
     pub fn populate_dynamic_buffers(&mut self, stmt: &Statement) -> QueryResult<()> {
         for (i, data) in self.data.iter_mut().enumerate() {
+            data.did_numeric_overflow_occur()?;
             // This is safe because we are re-binding the invalidated buffers
             // at the end of this function
             unsafe {
@@ -112,6 +114,10 @@ impl BindData {
         self.is_truncated.unwrap_or(0) != 0
     }
 
+    fn is_fixed_size_buffer(&self) -> bool {
+        known_buffer_size_for_ffi_type(self.tpe).is_some()
+    }
+
     fn bytes(&self) -> Option<&[u8]> {
         if self.is_null == 0 {
             Some(&*self.bytes)
@@ -166,6 +172,16 @@ impl BindData {
             None
         }
     }
+
+    fn did_numeric_overflow_occur(&self) -> QueryResult<()> {
+        use result::Error::DeserializationError;
+
+        if self.is_truncated() && self.is_fixed_size_buffer() {
+            Err(DeserializationError("Numeric overflow/underflow occurred".into()))
+        } else {
+            Ok(())
+        }
+    }
 }
 
 fn mysql_type_to_ffi_type(tpe: MysqlType) -> ffi::enum_field_types {

diesel/src/mysql/connection/mod.rs

@@ -1,6 +1,5 @@
 mod bind;
 mod raw;
-mod result;
 mod stmt;
 mod url;
 
@@ -62,7 +61,9 @@ impl Connection for MysqlConnection {
 
         let mut stmt = try!(self.prepare_query(&source.as_query()));
         stmt.execute()?;
-        stmt.results()?.map(|mut row| {
+        let mut metadata = Vec::new();
+        Mysql::row_metadata(&mut metadata);
+        stmt.results(metadata)?.map(|mut row| {
             U::Row::build_from_row(&mut row)
                 .map(U::build)
                 .map_err(DeserializationError)

diesel/src/mysql/connection/result.rs

@@ -1,24 +1,16 @@
 use super::{Statement, Binds, ffi, libc};
 use result::QueryResult;
 use row::Row;
-use mysql::Mysql;
+use mysql::{Mysql, MysqlType};
 
 pub struct StatementIterator<'a> {
     stmt: &'a mut Statement,
     output_binds: Binds,
 }
 
 impl<'a> StatementIterator<'a> {
-    pub fn new(stmt: &'a mut Statement) -> QueryResult<Self> {
-        use result::Error::QueryBuilderError;
-
-        let mut result_metadata = match try!(stmt.result_metadata()) {
-            Some(result) => result,
-            None => return Err(QueryBuilderError("Attempted to get results \
-                on a query with no results".into())),
-        };
-        let result_types = result_metadata.fields().map(|f| f.type_);
-        let mut output_binds = Binds::from_output_types(result_types);
+    pub fn new(stmt: &'a mut Statement, types: Vec<MysqlType>) -> QueryResult<Self> {
+        let mut output_binds = Binds::from_output_types(types);
 
         unsafe {
             output_binds.with_mysql_binds(|bind_ptr| stmt.bind_result(bind_ptr))?

diesel/src/mysql/connection/stmt/iterator.rs

@@ -9,7 +9,6 @@ use mysql::MysqlType;
 use result::{QueryResult, DatabaseErrorKind};
 use self::iterator::StatementIterator;
 use super::bind::Binds;
-use super::result::RawResult;
 
 pub struct Statement {
     stmt: *mut ffi::MYSQL_STMT,
@@ -56,17 +55,8 @@ impl Statement {
         affected_rows as usize
     }
 
-    pub fn results(&mut self) -> QueryResult<StatementIterator> {
-        StatementIterator::new(self)
-    }
-
-    fn result_metadata(&self) -> QueryResult<Option<RawResult>> {
-        let result = unsafe {
-            let result_ptr = ffi::mysql_stmt_result_metadata(self.stmt);
-            RawResult::from_raw(result_ptr)
-        };
-        self.did_an_error_occur()?;
-        Ok(result)
+    pub fn results(&mut self, types: Vec<MysqlType>) -> QueryResult<StatementIterator> {
+        StatementIterator::new(self, types)
     }
 
     fn last_error_message(&self) -> String {

diesel/src/mysql/connection/stmt/mod.rs

@@ -12,9 +12,13 @@ use types::{HasSqlType, FromSql, FromSqlRow, Nullable, ToSql, IsNull, NotNull};
 impl<T, DB> HasSqlType<Nullable<T>> for DB where
     DB: Backend + HasSqlType<T>, T: NotNull,
 {
-    fn metadata() -> DB::TypeMetadata{
+    fn metadata() -> DB::TypeMetadata {
         <DB as HasSqlType<T>>::metadata()
     }
+
+    fn row_metadata(out: &mut Vec<DB::TypeMetadata>) {
+        <DB as HasSqlType<T>>::row_metadata(out)
+    }
 }
 
 impl<T> QueryId for Nullable<T> where

diesel/src/types/impls/option.rs

@@ -23,6 +23,10 @@ macro_rules! tuple_impls {
                 fn metadata() -> DB::TypeMetadata {
                     unreachable!("Tuples should never implement `ToSql` directly");
                 }
+
+                fn row_metadata(out: &mut Vec<DB::TypeMetadata>) {
+                    $(<DB as HasSqlType<$T>>::row_metadata(out);)+
+                }
             }
 
             impl<$($T),+> NotNull for ($($T,)+) {

diesel/src/types/impls/tuples.rs

@@ -251,6 +251,10 @@ pub use pg::types::sql_types::*;
 
 pub trait HasSqlType<ST>: TypeMetadata {
     fn metadata() -> Self::TypeMetadata;
+
+    fn row_metadata(out: &mut Vec<Self::TypeMetadata>) {
+        out.push(Self::metadata())
+    }
 }
 
 pub trait NotNull {

diesel/src/types/mod.rs

@@ -16,6 +16,16 @@ fn adding_literal_to_column() {
     assert_eq!(Ok(expected_data), data);
 }
 
+#[test]
+#[cfg(not(feature="sqlite"))] // FIXME: Does SQLite provide a way to detect overflow?
+fn overflow_returns_an_error_but_does_not_panic() {
+    use schema::users::dsl::*;
+
+    let connection = connection_with_sean_and_tess_in_users_table();
+    let query_result = users.select(id + i32::max_value()).load::<i32>(&connection);
+    assert!(query_result.is_err(), "Integer overflow should have returned an error");
+}
+
 #[test]
 fn adding_column_to_column() {
     use schema::users::dsl::*;