Merge pull request #2931 from weiznich/cleanup/backend

Rewrite bind serialization layer

CHANGELOG.md

@@ -188,6 +188,11 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 
 * The MySQL connection is using the CLIENT_FOUND_ROWS from now on. This means that updating rows without changing any values will return the number of matched rows (like most other SQL servers do), as opposed to the number of changed rows.
 
+* The definition of `ToSql::to_sql` and `QueryFragment::walk_ast` has changed to allow serializing values without 
+  copying the value itself. This is useful for database backends like sqlite where you can directly share a buffer
+  with the database. Beside of the changed signature, existing impls of this trait should remain unchanged in almost 
+  all cases.
+
 ### Fixed
 
 * Many types were incorrectly considered non-aggregate when they should not

diesel/Cargo.toml

@@ -13,7 +13,7 @@ categories = ["database"]
 edition = "2018"
 
 [dependencies]
-byteorder = "1.0"
+byteorder = { version = "1.0", optional = true }
 chrono = { version = "0.4.19", optional = true, default-features = false, features = ["clock", "std"] }
 libc = { version = "0.2.0", optional = true }
 libsqlite3-sys = { version = ">=0.8.0, <0.24.0", optional = true, features = ["bundled_bindings"] }
@@ -44,7 +44,7 @@ ipnetwork = ">=0.12.2, <0.19.0"
 quickcheck = "1.0.3"
 
 [features]
-default = ["32-column-tables", "with-deprecated"]
+default = ["with-deprecated", "32-column-tables"]
 extras = ["chrono", "serde_json", "uuid", "network-address", "numeric", "r2d2"]
 unstable = ["diesel_derives/nightly"]
 large-tables = ["32-column-tables"]
@@ -59,8 +59,8 @@ without-deprecated = []
 with-deprecated = []
 network-address = ["ipnetwork", "libc"]
 numeric = ["num-bigint", "bigdecimal", "num-traits", "num-integer"]
-postgres_backend = ["diesel_derives/postgres", "bitflags"]
-mysql_backend = ["diesel_derives/mysql"]
+postgres_backend = ["diesel_derives/postgres", "bitflags", "byteorder"]
+mysql_backend = ["diesel_derives/mysql", "byteorder"]
 
 [package.metadata.docs.rs]
 features = ["postgres", "mysql", "sqlite", "extras"]

diesel/src/backend.rs

@@ -1,10 +1,6 @@
 //! Types which represent various database backends
-
-use byteorder::ByteOrder;
-
-use crate::query_builder::bind_collector::BindCollector;
 use crate::query_builder::QueryBuilder;
-use crate::sql_types::{self, HasSqlType};
+use crate::sql_types::{self, HasSqlType, TypeMetadata};
 
 /// A database backend
 ///
@@ -33,19 +29,24 @@ where
     Self: HasSqlType<sql_types::Time>,
     Self: HasSqlType<sql_types::Timestamp>,
     Self: for<'a> HasRawValue<'a>,
+    Self: for<'a> HasBindCollector<'a>,
 {
     /// The concrete `QueryBuilder` implementation for this backend.
     type QueryBuilder: QueryBuilder<Self>;
+}
+
+/// The bind collector type used to collect query binds for this backend
+///
+/// This trait is separate from `Backend` to imitate `type BindCollector<'a>`. It
+/// should only be referenced directly by implementors. Users of this type
+/// should instead use the [`BindCollector`] helper type instead.
+pub trait HasBindCollector<'a>: TypeMetadata + Sized {
     /// The concrete `BindCollector` implementation for this backend.
     ///
     /// Most backends should use [`RawBytesBindCollector`].
     ///
     /// [`RawBytesBindCollector`]: crate::query_builder::bind_collector::RawBytesBindCollector
-    type BindCollector: BindCollector<Self>;
-    /// What byte order is used to transmit integers?
-    ///
-    /// This type is only used if `RawValue` is `[u8]`.
-    type ByteOrder: ByteOrder;
+    type BindCollector: crate::query_builder::bind_collector::BindCollector<'a, Self> + 'a;
 }
 
 /// The raw representation of a database value given to `FromSql`.
@@ -60,19 +61,14 @@ pub trait HasRawValue<'a> {
     type RawValue;
 }
 
-/// A trait indicating that the provided raw value uses a binary representation internally
-// That's a false positive, `HasRawValue<'a>` is essentially
-// a reference wrapper
-#[allow(clippy::wrong_self_convention)]
-pub trait BinaryRawValue<'a>: HasRawValue<'a> {
-    /// Get the underlying binary representation of the raw value
-    fn as_bytes(value: Self::RawValue) -> &'a [u8];
-}
-
 /// A helper type to get the raw representation of a database type given to
 /// `FromSql`. Equivalent to `<DB as Backend>::RawValue<'a>`.
 pub type RawValue<'a, DB> = <DB as HasRawValue<'a>>::RawValue;
 
+/// A helper type to get the bind collector for a database backend.
+/// Equivalent to `<DB as HasBindCollector<'a>>::BindCollector<'a>`j
+pub type BindCollector<'a, DB> = <DB as HasBindCollector<'a>>::BindCollector;
+
 /// This trait provides various options to configure the
 /// generated SQL for a specific backend.
 ///

diesel/src/connection/mod.rs

@@ -29,20 +29,20 @@ pub trait SimpleConnection {
 /// implementation. This trait is only useful in combination with [`Connection`].
 ///
 /// Implementation wise this is a workaround for GAT's
-pub trait ConnectionGatWorkaround<'a, DB: Backend> {
+pub trait ConnectionGatWorkaround<'conn, 'query, DB: Backend> {
     /// The cursor type returned by [`Connection::load`]
     ///
     /// Users should handle this as opaque type that implements [`Iterator`]
     type Cursor: Iterator<Item = QueryResult<Self::Row>>;
     /// The row type used as [`Iterator::Item`] for the iterator implementation
     /// of [`ConnectionGatWorkaround::Cursor`]
-    type Row: crate::row::Row<'a, DB>;
+    type Row: crate::row::Row<'conn, DB>;
 }
 
 /// A connection to a database
 pub trait Connection: SimpleConnection + Sized + Send
 where
-    Self: for<'a> ConnectionGatWorkaround<'a, <Self as Connection>::Backend>,
+    Self: for<'a, 'b> ConnectionGatWorkaround<'a, 'b, <Self as Connection>::Backend>,
 {
     /// The backend this type connects to
     type Backend: Backend;
@@ -192,13 +192,13 @@ where
     fn execute(&mut self, query: &str) -> QueryResult<usize>;
 
     #[doc(hidden)]
-    fn load<T>(
-        &mut self,
+    fn load<'conn, 'query, T>(
+        &'conn mut self,
         source: T,
-    ) -> QueryResult<<Self as ConnectionGatWorkaround<Self::Backend>>::Cursor>
+    ) -> QueryResult<<Self as ConnectionGatWorkaround<'conn, 'query, Self::Backend>>::Cursor>
     where
         T: AsQuery,
-        T::Query: QueryFragment<Self::Backend> + QueryId,
+        T::Query: QueryFragment<Self::Backend> + QueryId + 'query,
         Self::Backend: QueryMetadata<T::SqlType>;
 
     #[doc(hidden)]

diesel/src/connection/statement_cache.rs

@@ -136,7 +136,7 @@ where
         source: &T,
         bind_types: &[DB::TypeMetadata],
         prepare_fn: F,
-    ) -> QueryResult<MaybeCached<Statement>>
+    ) -> QueryResult<MaybeCached<'_, Statement>>
     where
         T: QueryFragment<DB> + QueryId,
         F: FnOnce(&str, PrepareForCache) -> QueryResult<Statement>,
@@ -226,7 +226,7 @@ where
         }
     }
 
-    pub fn sql<T: QueryFragment<DB>>(&self, source: &T) -> QueryResult<Cow<str>> {
+    pub fn sql<T: QueryFragment<DB>>(&self, source: &T) -> QueryResult<Cow<'_, str>> {
         match *self {
             StatementCacheKey::Type(_) => Self::construct_sql(source).map(Cow::Owned),
             StatementCacheKey::Sql { ref sql, .. } => Ok(Cow::Borrowed(sql)),

diesel/src/deserialize.rs

@@ -411,7 +411,7 @@ pub use diesel_derives::QueryableByName;
 /// ```
 pub trait FromSql<A, DB: Backend>: Sized {
     /// See the trait documentation.
-    fn from_sql(bytes: backend::RawValue<DB>) -> Result<Self>;
+    fn from_sql(bytes: backend::RawValue<'_, DB>) -> Result<Self>;
 
     /// A specialized variant of `from_sql` for handling null values.
     ///
@@ -421,7 +421,7 @@ pub trait FromSql<A, DB: Backend>: Sized {
     /// If your custom type supports null values you need to provide a
     /// custom implementation.
     #[inline(always)]
-    fn from_nullable_sql(bytes: Option<backend::RawValue<DB>>) -> Result<Self> {
+    fn from_nullable_sql(bytes: Option<backend::RawValue<'_, DB>>) -> Result<Self> {
         match bytes {
             Some(bytes) => Self::from_sql(bytes),
             None => Err(Box::new(crate::result::UnexpectedNullError)),

diesel/src/expression/array_comparison.rs

@@ -1,12 +1,12 @@
 use crate::backend::sql_dialect;
 use crate::backend::Backend;
 use crate::backend::SqlDialect;
-use crate::expression::bound::Bound;
 use crate::expression::subselect::Subselect;
 use crate::expression::*;
 use crate::query_builder::*;
 use crate::query_builder::{BoxedSelectStatement, SelectStatement};
 use crate::result::QueryResult;
+use crate::serialize::ToSql;
 use crate::sql_types::Bool;
 use std::marker::PhantomData;
 
@@ -55,7 +55,7 @@ where
     DB: Backend,
     Self: QueryFragment<DB, DB::ArrayComparision>,
 {
-    fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         <Self as QueryFragment<DB, DB::ArrayComparision>>::walk_ast(self, pass)
     }
 }
@@ -68,7 +68,7 @@ where
     T: QueryFragment<DB>,
     U: QueryFragment<DB> + MaybeEmpty,
 {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         if self.values.is_empty() {
             out.push_sql("1=0");
         } else {
@@ -86,7 +86,7 @@ where
     DB: Backend,
     Self: QueryFragment<DB, DB::ArrayComparision>,
 {
-    fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         <Self as QueryFragment<DB, DB::ArrayComparision>>::walk_ast(self, pass)
     }
 }
@@ -99,7 +99,7 @@ where
     T: QueryFragment<DB>,
     U: QueryFragment<DB> + MaybeEmpty,
 {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         if self.values.is_empty() {
             out.push_sql("1=1");
         } else {
@@ -215,7 +215,7 @@ where
     Self: QueryFragment<DB, DB::ArrayComparision>,
     DB: Backend,
 {
-    fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         <Self as QueryFragment<DB, DB::ArrayComparision>>::walk_ast(self, pass)
     }
 }
@@ -224,11 +224,12 @@ impl<ST, I, DB> QueryFragment<DB, sql_dialect::array_comparision::AnsiSqlArrayCo
     for Many<ST, I>
 where
     DB: Backend
+        + HasSqlType<ST>
         + SqlDialect<ArrayComparision = sql_dialect::array_comparision::AnsiSqlArrayComparison>,
     ST: SingleValue,
-    for<'a> Bound<ST, &'a I>: QueryFragment<DB>,
+    I: ToSql<ST, DB>,
 {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         out.unsafe_to_cache_prepared();
         let mut first = true;
         for value in &self.0 {
@@ -237,7 +238,7 @@ where
             } else {
                 out.push_sql(", ");
             }
-            Bound::new(value).walk_ast(out.reborrow())?;
+            out.push_bind_param(value)?;
         }
         Ok(())
     }

diesel/src/expression/assume_not_null.rs

@@ -1,7 +1,6 @@
 use crate::backend::Backend;
 use crate::expression::TypedExpressionType;
 use crate::expression::*;
-use crate::query_builder::select_statement::NoFromClause;
 use crate::query_builder::*;
 use crate::query_source::joins::ToInnerJoin;
 use crate::result::QueryResult;
@@ -30,7 +29,7 @@ where
     DB: Backend,
     T: QueryFragment<DB>,
 {
-    fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         self.0.walk_ast(pass)
     }
 }

diesel/src/expression/bound.rs

@@ -16,7 +16,7 @@ pub struct Bound<T, U> {
 impl<T, U> Bound<T, U> {
     pub fn new(item: U) -> Self {
         Bound {
-            item: item,
+            item,
             _marker: PhantomData,
         }
     }
@@ -34,7 +34,7 @@ where
     DB: Backend + HasSqlType<T>,
     U: ToSql<T, DB>,
 {
-    fn walk_ast(&self, mut pass: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         pass.push_bind_param(&self.item)?;
         Ok(())
     }

diesel/src/expression/coerce.rs

@@ -60,7 +60,7 @@ where
     T: QueryFragment<DB>,
     DB: Backend,
 {
-    fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         self.expr.walk_ast(pass)
     }
 }

diesel/src/expression/count.rs

@@ -68,7 +68,7 @@ impl Expression for CountStar {
 }
 
 impl<DB: Backend> QueryFragment<DB> for CountStar {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         out.push_sql("COUNT(*)");
         Ok(())
     }
@@ -147,11 +147,11 @@ impl<T, E, DB> QueryFragment<DB> for CountDistinct<T, E>
 where
     T: SqlType + SingleValue,
     DB: Backend,
-    for<'a> &'a E: QueryFragment<DB>,
+    E: QueryFragment<DB>,
 {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         out.push_sql("COUNT(DISTINCT ");
-        (&self.expr).walk_ast(out.reborrow())?;
+        self.expr.walk_ast(out.reborrow())?;
         out.push_sql(")");
         Ok(())
     }

diesel/src/expression/exists.rs

@@ -55,7 +55,7 @@ where
     DB: Backend,
     Self: QueryFragment<DB, DB::ExistsSyntax>,
 {
-    fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, pass: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         <Self as QueryFragment<DB, DB::ExistsSyntax>>::walk_ast(self, pass)
     }
 }
@@ -65,7 +65,7 @@ where
     DB: Backend + SqlDialect<ExistsSyntax = sql_dialect::exists_syntax::AnsiSqlExistsSyntax>,
     T: QueryFragment<DB>,
 {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         out.push_sql("EXISTS (");
         self.0.walk_ast(out.reborrow())?;
         out.push_sql(")");

diesel/src/expression/functions/date_and_time.rs

@@ -17,7 +17,7 @@ impl Expression for now {
 }
 
 impl<DB: Backend> QueryFragment<DB> for now {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         out.push_sql("CURRENT_TIMESTAMP");
         Ok(())
     }
@@ -82,7 +82,7 @@ impl Expression for today {
 }
 
 impl<DB: Backend> QueryFragment<DB> for today {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         out.push_sql("CURRENT_DATE");
         Ok(())
     }

diesel/src/expression/functions/mod.rs

@@ -35,7 +35,8 @@ macro_rules! no_arg_sql_function_body {
         impl<DB> $crate::query_builder::QueryFragment<DB> for $type_name where
             DB: $crate::backend::Backend + $($constraint)::+,
         {
-            fn walk_ast(&self, mut out: $crate::query_builder::AstPass<DB>) -> $crate::result::QueryResult<()> {
+            fn walk_ast<'b>(&'b self, mut out: $crate::query_builder::AstPass<'_, 'b, DB>) -> $crate::result::QueryResult<()>
+            {
                 out.push_sql(concat!(stringify!($type_name), "()"));
                 Ok(())
             }
@@ -48,7 +49,7 @@ macro_rules! no_arg_sql_function_body {
         impl<DB> $crate::query_builder::QueryFragment<DB> for $type_name where
             DB: $crate::backend::Backend,
         {
-            fn walk_ast(&self, mut out: $crate::query_builder::AstPass<DB>) -> $crate::result::QueryResult<()> {
+            fn walk_ast<'b>(&'b self, mut out: $crate::query_builder::AstPass<'_, 'b, DB>) -> $crate::result::QueryResult<()> {
                 out.push_sql(concat!(stringify!($type_name), "()"));
                 Ok(())
             }

diesel/src/expression/grouped.rs

@@ -12,7 +12,7 @@ impl<T: Expression> Expression for Grouped<T> {
 }
 
 impl<T: QueryFragment<DB>, DB: Backend> QueryFragment<DB> for Grouped<T> {
-    fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
+    fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         out.push_sql("(");
         self.0.walk_ast(out.reborrow())?;
         out.push_sql(")");