Merge pull request diesel-rs#1751 from diesel-rs/sg-fix-mysql-unsigned-no-breaking-change

Fix our handling of unsigned types on MySQL

Author: sgrif

diesel/src/mysql/backend.rs

@@ -2,9 +2,9 @@
 
 use byteorder::NativeEndian;
 
+use super::bind_collector::MysqlBindCollector;
 use super::query_builder::MysqlQueryBuilder;
 use backend::*;
-use query_builder::bind_collector::RawBytesBindCollector;
 use sql_types::TypeMetadata;
 
 /// The MySQL backend
@@ -48,7 +48,7 @@ pub enum MysqlType {
 
 impl Backend for Mysql {
     type QueryBuilder = MysqlQueryBuilder;
-    type BindCollector = RawBytesBindCollector<Mysql>;
+    type BindCollector = MysqlBindCollector;
     type RawValue = [u8];
     type ByteOrder = NativeEndian;
 }

diesel/src/mysql/bind_collector.rs

@@ -0,0 +1,38 @@
+use super::{Mysql, MysqlType};
+use query_builder::BindCollector;
+use result::Error::SerializationError;
+use result::QueryResult;
+use serialize::{IsNull, Output, ToSql};
+use sql_types::{HasSqlType, IsSigned};
+
+#[derive(Default)]
+#[doc(hidden)]
+#[allow(missing_debug_implementations)]
+pub struct MysqlBindCollector {
+    pub(crate) binds: Vec<(MysqlType, IsSigned, Option<Vec<u8>>)>,
+}
+
+impl MysqlBindCollector {
+    pub(crate) fn new() -> Self {
+        Self::default()
+    }
+}
+
+impl BindCollector<Mysql> for MysqlBindCollector {
+    fn push_bound_value<T, U>(&mut self, bind: &U, metadata_lookup: &()) -> QueryResult<()>
+    where
+        Mysql: HasSqlType<T>,
+        U: ToSql<T, Mysql>,
+    {
+        let mut to_sql_output = Output::new(Vec::new(), metadata_lookup);
+        let is_null = bind.to_sql(&mut to_sql_output).map_err(SerializationError)?;
+        let bytes = match is_null {
+            IsNull::No => Some(to_sql_output.into_inner()),
+            IsNull::Yes => None,
+        };
+        let metadata = Mysql::metadata(metadata_lookup);
+        let sign = Mysql::is_signed();
+        self.binds.push((metadata, sign, bytes));
+        Ok(())
+    }
+}

diesel/src/mysql/connection/bind.rs

@@ -6,6 +6,7 @@ use std::os::raw as libc;
 use super::stmt::Statement;
 use mysql::MysqlType;
 use result::QueryResult;
+use sql_types::IsSigned;
 
 pub struct Binds {
     data: Vec<BindData>,
@@ -14,20 +15,20 @@ pub struct Binds {
 impl Binds {
     pub fn from_input_data<Iter>(input: Iter) -> Self
     where
-        Iter: IntoIterator<Item = (MysqlType, Option<Vec<u8>>)>,
+        Iter: IntoIterator<Item = (MysqlType, IsSigned, Option<Vec<u8>>)>,
     {
         let data = input
             .into_iter()
-            .map(|(tpe, bytes)| BindData::for_input(tpe, bytes))
+            .map(|(tpe, sign, bytes)| BindData::for_input(tpe, is_signed_to_my_bool(sign), bytes))
             .collect();
 
         Binds { data: data }
     }
 
-    pub fn from_output_types(types: Vec<MysqlType>) -> Self {
+    pub fn from_output_types(types: Vec<(MysqlType, IsSigned)>) -> Self {
         let data = types
             .into_iter()
-            .map(mysql_type_to_ffi_type)
+            .map(|(ty, sign)| (mysql_type_to_ffi_type(ty), is_signed_to_my_bool(sign)))
             .map(BindData::for_output)
             .collect();
 
@@ -37,7 +38,7 @@ impl Binds {
     pub fn from_result_metadata(fields: &[ffi::MYSQL_FIELD]) -> Self {
         let data = fields
             .iter()
-            .map(|field| field.type_)
+            .map(|field| (field.type_, is_field_unsigned(field)))
             .map(BindData::for_output)
             .collect();
 
@@ -89,10 +90,11 @@ struct BindData {
     length: libc::c_ulong,
     is_null: ffi::my_bool,
     is_truncated: Option<ffi::my_bool>,
+    is_unsigned: ffi::my_bool,
 }
 
 impl BindData {
-    fn for_input(tpe: MysqlType, data: Option<Vec<u8>>) -> Self {
+    fn for_input(tpe: MysqlType, is_unsigned: ffi::my_bool, data: Option<Vec<u8>>) -> Self {
         let is_null = if data.is_none() { 1 } else { 0 };
         let bytes = data.unwrap_or_default();
         let length = bytes.len() as libc::c_ulong;
@@ -103,10 +105,11 @@ impl BindData {
             length: length,
             is_null: is_null,
             is_truncated: None,
+            is_unsigned,
         }
     }
 
-    fn for_output(tpe: ffi::enum_field_types) -> Self {
+    fn for_output((tpe, is_unsigned): (ffi::enum_field_types, ffi::my_bool)) -> Self {
         let bytes = known_buffer_size_for_ffi_type(tpe)
             .map(|len| vec![0; len])
             .unwrap_or_default();
@@ -118,6 +121,7 @@ impl BindData {
             length: length,
             is_null: 0,
             is_truncated: Some(0),
+            is_unsigned,
         }
     }
 
@@ -151,6 +155,7 @@ impl BindData {
         bind.buffer_length = self.bytes.capacity() as libc::c_ulong;
         bind.length = &mut self.length;
         bind.is_null = &mut self.is_null;
+        bind.is_unsigned = self.is_unsigned;
 
         if let Some(ref mut is_truncated) = self.is_truncated {
             bind.error = is_truncated;
@@ -236,3 +241,15 @@ fn known_buffer_size_for_ffi_type(tpe: ffi::enum_field_types) -> Option<usize> {
         _ => None,
     }
 }
+
+fn is_field_unsigned(field: &ffi::MYSQL_FIELD) -> ffi::my_bool {
+    const UNSIGNED_FLAG: libc::c_uint = 32;
+    (field.flags & UNSIGNED_FLAG > 0) as _
+}
+
+fn is_signed_to_my_bool(sign: IsSigned) -> ffi::my_bool {
+    match sign {
+        IsSigned::Signed => false as _,
+        IsSigned::Unsigned => true as _,
+    }
+}

diesel/src/mysql/connection/mod.rs

@@ -7,9 +7,9 @@ use self::raw::RawConnection;
 use self::stmt::Statement;
 use self::url::ConnectionOptions;
 use super::backend::Mysql;
+use super::bind_collector::MysqlBindCollector;
 use connection::*;
 use deserialize::{Queryable, QueryableByName};
-use query_builder::bind_collector::RawBytesBindCollector;
 use query_builder::*;
 use result::*;
 use sql_types::HasSqlType;
@@ -71,7 +71,7 @@ impl Connection for MysqlConnection {
 
         let mut stmt = try!(self.prepare_query(&source.as_query()));
         let mut metadata = Vec::new();
-        Mysql::row_metadata(&mut metadata, &());
+        Mysql::mysql_row_metadata(&mut metadata, &());
         let results = unsafe { stmt.results(metadata)? };
         results.map(|mut row| {
             U::Row::build_from_row(&mut row)
@@ -118,11 +118,9 @@ impl MysqlConnection {
     {
         let mut stmt = self.statement_cache
             .cached_statement(source, &[], |sql| self.raw_connection.prepare(sql))?;
-        let mut bind_collector = RawBytesBindCollector::<Mysql>::new();
+        let mut bind_collector = MysqlBindCollector::new();
         try!(source.collect_binds(&mut bind_collector, &()));
-        let metadata = bind_collector.metadata;
-        let binds = bind_collector.binds;
-        try!(stmt.bind(metadata.into_iter().zip(binds)));
+        try!(stmt.bind(bind_collector.binds));
         Ok(stmt)
     }
 

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

@@ -4,6 +4,7 @@ use super::{ffi, libc, Binds, Statement, StatementMetadata};
 use mysql::{Mysql, MysqlType};
 use result::QueryResult;
 use row::*;
+use sql_types::IsSigned;
 
 pub struct StatementIterator<'a> {
     stmt: &'a mut Statement,
@@ -12,7 +13,7 @@ pub struct StatementIterator<'a> {
 
 #[cfg_attr(feature = "clippy", allow(should_implement_trait))] // don't neet `Iterator` here
 impl<'a> StatementIterator<'a> {
-    pub fn new(stmt: &'a mut Statement, types: Vec<MysqlType>) -> QueryResult<Self> {
+    pub fn new(stmt: &'a mut Statement, types: Vec<(MysqlType, IsSigned)>) -> QueryResult<Self> {
         let mut output_binds = Binds::from_output_types(types);
 
         execute_statement(stmt, &mut output_binds)?;

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

@@ -11,6 +11,7 @@ use self::metadata::*;
 use super::bind::Binds;
 use mysql::MysqlType;
 use result::{DatabaseErrorKind, QueryResult};
+use sql_types::IsSigned;
 use util::NonNull;
 
 pub struct Statement {
@@ -39,7 +40,7 @@ impl Statement {
 
     pub fn bind<Iter>(&mut self, binds: Iter) -> QueryResult<()>
     where
-        Iter: IntoIterator<Item = (MysqlType, Option<Vec<u8>>)>,
+        Iter: IntoIterator<Item = (MysqlType, IsSigned, Option<Vec<u8>>)>,
     {
         let mut input_binds = Binds::from_input_data(binds);
         input_binds.with_mysql_binds(|bind_ptr| {
@@ -72,7 +73,10 @@ impl Statement {
     /// This function should be called instead of `execute` for queries which
     /// have a return value. After calling this function, `execute` can never
     /// be called on this statement.
-    pub unsafe fn results(&mut self, types: Vec<MysqlType>) -> QueryResult<StatementIterator> {
+    pub unsafe fn results(
+        &mut self,
+        types: Vec<(MysqlType, IsSigned)>,
+    ) -> QueryResult<StatementIterator> {
         StatementIterator::new(self, types)
     }
 

diesel/src/mysql/mod.rs

@@ -5,6 +5,7 @@
 //! MySQL, you may need to work with this module directly.
 
 mod backend;
+mod bind_collector;
 mod connection;
 
 mod query_builder;

diesel/src/mysql/types/mod.rs

@@ -88,6 +88,10 @@ where
     fn metadata(lookup: &()) -> MysqlType {
         <Mysql as HasSqlType<ST>>::metadata(lookup)
     }
+
+    fn is_signed() -> IsSigned {
+        IsSigned::Unsigned
+    }
 }
 
 /// Represents the MySQL datetime type.

diesel/src/sql_types/mod.rs

@@ -401,17 +401,44 @@ pub trait HasSqlType<ST>: TypeMetadata {
     /// of this method should not do dynamic lookup unless absolutely necessary
     fn metadata(lookup: &Self::MetadataLookup) -> Self::TypeMetadata;
 
-    /// Fetch the metadata for a tuple representing an entire row
-    ///
-    /// The default implementation of this method simply calls `Self::metadata`.
-    /// You generally should not need to override this method.
-    ///
-    /// However, if you are writing an implementation of `HasSqlType` that
-    /// simply delegates to an inner type (for example, `Nullable` does this),
-    /// then you should ensure that you delegate this method as well.
+    #[doc(hidden)]
+    #[cfg(feature = "with-deprecated")]
+    #[deprecated(
+        since = "1.4.0",
+        note = "This method is no longer used, and has been deprecated without replacement"
+    )]
     fn row_metadata(out: &mut Vec<Self::TypeMetadata>, lookup: &Self::MetadataLookup) {
         out.push(Self::metadata(lookup))
     }
+
+    #[doc(hidden)]
+    #[cfg(feature = "mysql")]
+    fn is_signed() -> IsSigned {
+        IsSigned::Signed
+    }
+
+    #[doc(hidden)]
+    #[cfg(feature = "mysql")]
+    fn mysql_metadata(lookup: &Self::MetadataLookup) -> (Self::TypeMetadata, IsSigned) {
+        (Self::metadata(lookup), Self::is_signed())
+    }
+
+    #[doc(hidden)]
+    #[cfg(feature = "mysql")]
+    fn mysql_row_metadata(
+        out: &mut Vec<(Self::TypeMetadata, IsSigned)>,
+        lookup: &Self::MetadataLookup,
+    ) {
+        out.push(Self::mysql_metadata(lookup))
+    }
+}
+
+#[doc(hidden)]
+#[cfg(feature = "mysql")]
+#[derive(Debug, Clone, Copy)]
+pub enum IsSigned {
+    Signed,
+    Unsigned,
 }
 
 /// Information about how a backend stores metadata about given SQL types

diesel/src/type_impls/option.rs

@@ -10,6 +10,9 @@ use row::NamedRow;
 use serialize::{self, IsNull, Output, ToSql};
 use sql_types::{HasSqlType, NotNull, Nullable};
 
+#[cfg(feature = "mysql")]
+use sql_types::IsSigned;
+
 impl<T, DB> HasSqlType<Nullable<T>> for DB
 where
     DB: Backend + HasSqlType<T>,
@@ -19,9 +22,19 @@ where
         <DB as HasSqlType<T>>::metadata(lookup)
     }
 
+    #[cfg(feature = "with-deprecated")]
+    #[allow(deprecated)]
     fn row_metadata(out: &mut Vec<DB::TypeMetadata>, lookup: &DB::MetadataLookup) {
         <DB as HasSqlType<T>>::row_metadata(out, lookup)
     }
+
+    #[cfg(feature = "mysql")]
+    fn mysql_row_metadata(
+        out: &mut Vec<(DB::TypeMetadata, IsSigned)>,
+        lookup: &DB::MetadataLookup,
+    ) {
+        <DB as HasSqlType<T>>::mysql_row_metadata(out, lookup)
+    }
 }
 
 impl<T> QueryId for Nullable<T>