Interacting with the database requires a mutable connection

This commit changes all connection implementations to take `&mut self`
on all methods that interact with the database.
This unblocks the following features:

* Returnig a cursor/iterator from a `RunQueryDsl` method
* Removing some unsafe code and interior mutability inside the current
  connection implementations
* Possibly the first step of a async `Connection` variant to workaround
  the `Future` not `Send` issues there.

CHANGELOG.md

@@ -169,6 +169,8 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 * The `#[table_name]` attribute for derive macros can now refer to any path and is no
   longer limited to identifiers from the current scope.
 
+* Interacting with a database requires a mutable connection. 
+
 ### Fixed
 
 * Many types were incorrectly considered non-aggregate when they should not

bin/test

@@ -27,23 +27,23 @@ else
   (cd diesel_derives && cargo test --features "diesel/sqlite" $*)
   (cd diesel_tests && cargo test --features "sqlite" --no-default-features $*)
   (cd diesel_dynamic_schema && cargo test --features "sqlite" $*)
-  (cd diesel_bench && cargo test --features "sqlite" $*)
+  (cd diesel_bench && cargo bench --features "sqlite" --list $*)
 
   (cd diesel_migrations && cargo test --features "postgres diesel/postgres" $*)
   (cd diesel_migrations/migrations_macros && cargo test --features "diesel/postgres postgres" $*)
   (cd diesel_derives && cargo test --features "diesel/postgres" $*)
   (cd diesel_cli && cargo test --features "postgres" --no-default-features $*)
   (cd diesel_tests && cargo test --features "postgres" --no-default-features $*)
   (cd diesel_dynamic_schema && cargo test --features "postgres" $*)
-  (cd diesel_bench && cargo test --features "postgres" $*)
+  (cd diesel_bench && cargo bench --features "postgres" --list $*)
 
   (cd diesel_migrations && cargo test --features "mysql diesel/mysql" $* -- --test-threads 1)
   (cd diesel_migrations/migrations_macros && cargo test --features "diesel/mysql mysql" $* -- --test-threads 1)
   (cd diesel_derives && cargo test --features "diesel/mysql" $* -- --test-threads 1)
   (cd diesel_cli && cargo test --features "mysql" --no-default-features $* -- --test-threads 1)
   (cd diesel_tests && cargo test --features "mysql" --no-default-features $* -- --test-threads 1)
   (cd diesel_dynamic_schema && cargo test --features "mysql" $* -- --test-threads 1)
-  (cd diesel_bench && cargo test --features "mysql" $* -- --test-threads 1)
+  (cd diesel_bench && cargo bench --features "mysql" --list $* -- --test-threads 1)
 
   (cd diesel_compile_tests && cargo test $*)
   (cd diesel_migrations/migrations_internals && cargo test $*)

diesel/Cargo.toml

@@ -44,7 +44,7 @@ ipnetwork = ">=0.12.2, <0.19.0"
 quickcheck = "0.9"
 
 [features]
-default = ["with-deprecated", "32-column-tables"]
+default = ["with-deprecated", "32-column-tables", "r2d2"]
 extras = ["chrono", "serde_json", "uuid", "network-address", "numeric", "r2d2"]
 unstable = ["diesel_derives/nightly"]
 large-tables = ["32-column-tables"]

diesel/src/associations/belongs_to.rs

@@ -70,10 +70,10 @@ pub trait BelongsTo<Parent> {
 /// # }
 /// #
 /// # fn run_test() -> QueryResult<()> {
-/// #     let connection = establish_connection();
-/// let users = users::table.load::<User>(&connection)?;
+/// #     let mut connection = establish_connection();
+/// let users = users::table.load::<User>(&mut connection)?;
 /// let posts = Post::belonging_to(&users)
-///     .load::<Post>(&connection)?
+///     .load::<Post>(&mut connection)?
 ///     .grouped_by(&users);
 /// let data = users.into_iter().zip(posts).collect::<Vec<_>>();
 ///

diesel/src/associations/mod.rs

@@ -29,11 +29,11 @@
 //! # }
 //! #
 //! # fn run_test() -> QueryResult<()> {
-//! #     let connection = establish_connection();
+//! #     let mut connection = establish_connection();
 //! #     use self::users::dsl::*;
-//! let user = users.find(2).get_result::<User>(&connection)?;
+//! let user = users.find(2).get_result::<User>(&mut connection)?;
 //! let users_post = Post::belonging_to(&user)
-//!     .first(&connection)?;
+//!     .first(&mut connection)?;
 //! let expected = Post { id: 3, user_id: 2, title: "My first post too".into() };
 //! assert_eq!(expected, users_post);
 //! #     Ok(())
@@ -115,11 +115,11 @@
 //! #
 //! # fn main() {
 //! #   use self::users::dsl::*;
-//! #   let connection = establish_connection();
+//! #   let mut connection = establish_connection();
 //! #
-//! let user = users.find(1).first::<User>(&connection).expect("Error loading user");
+//! let user = users.find(1).first::<User>(&mut connection).expect("Error loading user");
 //! let post_list = Post::belonging_to(&user)
-//!     .load::<Post>(&connection)
+//!     .load::<Post>(&mut connection)
 //!     .expect("Error loading posts");
 //! let expected = vec![
 //!     Post { id: 1, user_id: 1, title: "My first post".to_string() },
@@ -172,21 +172,21 @@
 //! # }
 //! #
 //! # fn run_test() -> QueryResult<()> {
-//! #     let connection = establish_connection();
+//! #     let mut connection = establish_connection();
 //! #     use self::users::dsl::*;
 //! #     use self::posts::dsl::{posts, title};
-//! let sean = users.filter(name.eq("Sean")).first::<User>(&connection)?;
-//! let tess = users.filter(name.eq("Tess")).first::<User>(&connection)?;
+//! let sean = users.filter(name.eq("Sean")).first::<User>(&mut connection)?;
+//! let tess = users.filter(name.eq("Tess")).first::<User>(&mut connection)?;
 //!
 //! let seans_posts = Post::belonging_to(&sean)
 //!     .select(title)
-//!     .load::<String>(&connection)?;
+//!     .load::<String>(&mut connection)?;
 //! assert_eq!(vec!["My first post", "About Rust"], seans_posts);
 //!
 //! // A vec or slice can be passed as well
 //! let more_posts = Post::belonging_to(&vec![sean, tess])
 //!     .select(title)
-//!     .load::<String>(&connection)?;
+//!     .load::<String>(&mut connection)?;
 //! assert_eq!(vec!["My first post", "About Rust", "My first post too"], more_posts);
 //! #     Ok(())
 //! # }
@@ -226,10 +226,10 @@
 //! # }
 //! #
 //! # fn run_test() -> QueryResult<()> {
-//! #     let connection = establish_connection();
-//! let users = users::table.load::<User>(&connection)?;
+//! #     let mut connection = establish_connection();
+//! let users = users::table.load::<User>(&mut connection)?;
 //! let posts = Post::belonging_to(&users)
-//!     .load::<Post>(&connection)?
+//!     .load::<Post>(&mut connection)?
 //!     .grouped_by(&users);
 //! let data = users.into_iter().zip(posts).collect::<Vec<_>>();
 //!
@@ -290,15 +290,15 @@
 //! # }
 //! #
 //! # fn main() {
-//! #   let connection = establish_connection();
+//! #   let mut connection = establish_connection();
 //! #
-//! let users: Vec<User> = users::table.load::<User>(&connection)
+//! let users: Vec<User> = users::table.load::<User>(&mut connection)
 //!     .expect("error loading users");
 //! let posts: Vec<Post> = Post::belonging_to(&users)
-//!     .load::<Post>(&connection)
+//!     .load::<Post>(&mut connection)
 //!     .expect("error loading posts");
 //! let comments: Vec<Comment> = Comment::belonging_to(&posts)
-//!     .load::<Comment>(&connection)
+//!     .load::<Comment>(&mut connection)
 //!     .expect("Error loading comments");
 //! let grouped_comments: Vec<Vec<Comment>> = comments.grouped_by(&posts);
 //! let posts_and_comments: Vec<Vec<(Post, Vec<Comment>)>> = posts

diesel/src/connection/mod.rs

@@ -14,7 +14,9 @@ use crate::result::*;
 
 #[doc(hidden)]
 pub use self::statement_cache::{MaybeCached, StatementCache, StatementCacheKey};
-pub use self::transaction_manager::{AnsiTransactionManager, TransactionManager};
+pub use self::transaction_manager::{
+    AnsiTransactionManager, AnsiTransactionManagerData, TransactionManager,
+};
 
 /// Perform simple operations on a backend.
 ///
@@ -24,22 +26,25 @@ pub trait SimpleConnection {
     ///
     /// This function is used to execute migrations,
     /// which may contain more than one SQL statement.
-    fn batch_execute(&self, query: &str) -> QueryResult<()>;
+    fn batch_execute(&mut self, query: &str) -> QueryResult<()>;
 }
 
 /// A connection to a database
-pub trait Connection: SimpleConnection + Send {
+pub trait Connection: SimpleConnection + Sized + Send {
     /// The backend this type connects to
     type Backend: Backend;
 
+    #[doc(hidden)]
+    type TransactionManager: TransactionManager<Self>;
+    #[doc(hidden)]
+    type TransactionData;
+
     /// Establishes a new connection to the database
     ///
     /// The argument to this method and the method's behavior varies by backend.
     /// See the documentation for that backend's connection class
     /// for details about what it accepts and how it behaves.
-    fn establish(database_url: &str) -> ConnectionResult<Self>
-    where
-        Self: Sized;
+    fn establish(database_url: &str) -> ConnectionResult<Self>;
 
     /// Executes the given function inside of a database transaction
     ///
@@ -69,65 +74,59 @@ pub trait Connection: SimpleConnection + Send {
     /// #
     /// # fn run_test() -> QueryResult<()> {
     /// #     use schema::users::dsl::*;
-    /// #     let conn = establish_connection();
-    /// conn.transaction::<_, Error, _>(|| {
+    /// #     let mut conn = establish_connection();
+    /// conn.transaction::<_, Error, _>(|conn| {
     ///     diesel::insert_into(users)
     ///         .values(name.eq("Ruby"))
-    ///         .execute(&conn)?;
+    ///         .execute(conn)?;
     ///
-    ///     let all_names = users.select(name).load::<String>(&conn)?;
+    ///     let all_names = users.select(name).load::<String>(conn)?;
     ///     assert_eq!(vec!["Sean", "Tess", "Ruby"], all_names);
     ///
     ///     Ok(())
     /// })?;
     ///
-    /// conn.transaction::<(), _, _>(|| {
+    /// conn.transaction::<(), _, _>(|conn| {
     ///     diesel::insert_into(users)
     ///         .values(name.eq("Pascal"))
-    ///         .execute(&conn)?;
+    ///         .execute(conn)?;
     ///
-    ///     let all_names = users.select(name).load::<String>(&conn)?;
+    ///     let all_names = users.select(name).load::<String>(conn)?;
     ///     assert_eq!(vec!["Sean", "Tess", "Ruby", "Pascal"], all_names);
     ///
     ///     // If we want to roll back the transaction, but don't have an
     ///     // actual error to return, we can return `RollbackTransaction`.
     ///     Err(Error::RollbackTransaction)
     /// });
     ///
-    /// let all_names = users.select(name).load::<String>(&conn)?;
+    /// let all_names = users.select(name).load::<String>(&mut conn)?;
     /// assert_eq!(vec!["Sean", "Tess", "Ruby"], all_names);
     /// #     Ok(())
     /// # }
     /// ```
-    fn transaction<T, E, F>(&self, f: F) -> Result<T, E>
+    fn transaction<T, E, F>(&mut self, f: F) -> Result<T, E>
     where
-        Self: Sized,
-        F: FnOnce() -> Result<T, E>,
+        F: FnOnce(&mut Self) -> Result<T, E>,
         E: From<Error>,
     {
-        let transaction_manager = self.transaction_manager();
-        transaction_manager.begin_transaction(self)?;
-        match f() {
+        Self::TransactionManager::begin_transaction(self)?;
+        match f(&mut *self) {
             Ok(value) => {
-                transaction_manager.commit_transaction(self)?;
+                Self::TransactionManager::commit_transaction(self)?;
                 Ok(value)
             }
             Err(e) => {
-                transaction_manager.rollback_transaction(self)?;
+                Self::TransactionManager::rollback_transaction(self)?;
                 Err(e)
             }
         }
     }
 
     /// Creates a transaction that will never be committed. This is useful for
     /// tests. Panics if called while inside of a transaction.
-    fn begin_test_transaction(&self) -> QueryResult<()>
-    where
-        Self: Sized,
-    {
-        let transaction_manager = self.transaction_manager();
-        assert_eq!(transaction_manager.get_transaction_depth(), 0);
-        transaction_manager.begin_transaction(self)
+    fn begin_test_transaction(&mut self) -> QueryResult<()> {
+        assert_eq!(Self::TransactionManager::get_transaction_depth(self), 0);
+        Self::TransactionManager::begin_transaction(self)
     }
 
     /// Executes the given function inside a transaction, but does not commit
@@ -145,61 +144,56 @@ pub trait Connection: SimpleConnection + Send {
     /// #
     /// # fn run_test() -> QueryResult<()> {
     /// #     use schema::users::dsl::*;
-    /// #     let conn = establish_connection();
-    /// conn.test_transaction::<_, Error, _>(|| {
+    /// #     let mut conn = establish_connection();
+    /// conn.test_transaction::<_, Error, _>(|conn| {
     ///     diesel::insert_into(users)
     ///         .values(name.eq("Ruby"))
-    ///         .execute(&conn)?;
+    ///         .execute(conn)?;
     ///
-    ///     let all_names = users.select(name).load::<String>(&conn)?;
+    ///     let all_names = users.select(name).load::<String>(conn)?;
     ///     assert_eq!(vec!["Sean", "Tess", "Ruby"], all_names);
     ///
     ///     Ok(())
     /// });
     ///
     /// // Even though we returned `Ok`, the transaction wasn't committed.
-    /// let all_names = users.select(name).load::<String>(&conn)?;
+    /// let all_names = users.select(name).load::<String>(&mut conn)?;
     /// assert_eq!(vec!["Sean", "Tess"], all_names);
     /// #     Ok(())
     /// # }
     /// ```
-    fn test_transaction<T, E, F>(&self, f: F) -> T
+    fn test_transaction<T, E, F>(&mut self, f: F) -> T
     where
-        F: FnOnce() -> Result<T, E>,
+        F: FnOnce(&mut Self) -> Result<T, E>,
         E: Debug,
-        Self: Sized,
     {
         let mut user_result = None;
-        let _ = self.transaction::<(), _, _>(|| {
-            user_result = f().ok();
+        let _ = self.transaction::<(), _, _>(|conn| {
+            user_result = f(conn).ok();
             Err(Error::RollbackTransaction)
         });
         user_result.expect("Transaction did not succeed")
     }
 
     #[doc(hidden)]
-    fn execute(&self, query: &str) -> QueryResult<usize>;
+    fn execute(&mut self, query: &str) -> QueryResult<usize>;
 
     #[doc(hidden)]
-    fn load<T, U, ST>(&self, source: T) -> QueryResult<Vec<U>>
+    fn load<T, U, ST>(&mut self, source: T) -> QueryResult<Vec<U>>
     where
-        Self: Sized,
         T: AsQuery,
         T::Query: QueryFragment<Self::Backend> + QueryId,
         T::SqlType: CompatibleType<U, Self::Backend, SqlType = ST>,
         U: FromSqlRow<ST, Self::Backend>,
         Self::Backend: QueryMetadata<T::SqlType>;
 
     #[doc(hidden)]
-    fn execute_returning_count<T>(&self, source: &T) -> QueryResult<usize>
+    fn execute_returning_count<T>(&mut self, source: &T) -> QueryResult<usize>
     where
-        Self: Sized,
         T: QueryFragment<Self::Backend> + QueryId;
 
     #[doc(hidden)]
-    fn transaction_manager(&self) -> &dyn TransactionManager<Self>
-    where
-        Self: Sized;
+    fn transaction_state(&mut self) -> &mut Self::TransactionData;
 }
 
 /// A variant of the [`Connection`](trait.Connection.html) trait that is
@@ -210,7 +204,7 @@ pub trait Connection: SimpleConnection + Send {
 /// this trait won't help you much. Normally you should only
 /// need to use this trait if you are interacting with a connection
 /// passed to a [`Migration`](../migration/trait.Migration.html)
-pub trait BoxableConnection<DB: Backend>: Connection<Backend = DB> + std::any::Any {
+pub trait BoxableConnection<DB: Backend>: SimpleConnection + std::any::Any {
     #[doc(hidden)]
     fn as_any(&self) -> &dyn std::any::Any;
 }