Unify InsertStatement and InsertQuery into a single struct

I'm working on adding support for batch insert on SQLite, and fixing the
insert empty slice bug (diesel-rs#384). My approach is to override `ExecuteDsl`
and `LoadDsl` at the point where `InsertStatement` currently lives.
I expect that we'll have `into` perform some polymorphic dispatch to get
back either a `SingleInsertStatement` or `BatchInsertStatement`. If we
also have `InsertQuery` on top of that, the amount of redundant structs
and impls will get quite large. This change should make that refactoring
much easier.

I also took this opportunity to make sure our compile tests were up to
snuff around this, and made sure that the tests failed in the expected
fashion when I introduced some mistakes as part of this refactor.

Author: sgrif

diesel/src/query_builder/clause_macro.rs

@@ -1,5 +1,9 @@
 macro_rules! simple_clause {
     ($no_clause:ident, $clause:ident, $sql:expr) => {
+        simple_clause!($no_clause, $clause, $sql, backend_bounds = );
+    };
+
+    ($no_clause:ident, $clause:ident, $sql:expr, backend_bounds = $($backend_bounds:ident),*) => {
         use backend::Backend;
         use result::QueryResult;
         use super::{QueryFragment, QueryBuilder, BuildQueryResult};
@@ -27,7 +31,7 @@ macro_rules! simple_clause {
         pub struct $clause<Expr>(pub Expr);
 
         impl<Expr, DB> QueryFragment<DB> for $clause<Expr> where
-            DB: Backend,
+            DB: Backend $(+ $backend_bounds)*,
             Expr: QueryFragment<DB>,
         {
             fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {

diesel/src/query_builder/insert_statement.rs

@@ -1,9 +1,10 @@
-use backend::{Backend, SupportsReturningClause};
+use backend::Backend;
 use expression::{Expression, SelectableExpression, NonAggregate};
 use persistable::{Insertable, InsertValues};
 use query_builder::*;
 use query_source::Table;
 use result::QueryResult;
+use super::returning_clause::*;
 
 /// The structure returned by [`insert`](fn.insert.html). The only thing that can be done with it
 /// is call `into`.
@@ -30,23 +31,26 @@ impl<T, Op> IncompleteInsertStatement<T, Op> {
             operator: self.operator,
             target: target,
             records: self.records,
+            returning: NoReturningClause,
         }
     }
 }
 
 #[derive(Debug, Copy, Clone)]
-pub struct InsertStatement<T, U, Op> {
+pub struct InsertStatement<T, U, Op, Ret=NoReturningClause> {
     operator: Op,
     target: T,
     records: U,
+    returning: Ret,
 }
 
-impl<T, U, Op, DB> QueryFragment<DB> for InsertStatement<T, U, Op> where
+impl<T, U, Op, Ret, DB> QueryFragment<DB> for InsertStatement<T, U, Op, Ret> where
     DB: Backend,
     T: Table,
     T::FromClause: QueryFragment<DB>,
     U: Insertable<T, DB> + Copy,
     Op: QueryFragment<DB>,
+    Ret: QueryFragment<DB>,
 {
     fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
         let values = self.records.values();
@@ -57,6 +61,7 @@ impl<T, U, Op, DB> QueryFragment<DB> for InsertStatement<T, U, Op> where
         try!(values.column_names(out));
         out.push_sql(") VALUES ");
         try!(values.values_clause(out));
+        try!(self.returning.to_sql(out));
         Ok(())
     }
 
@@ -65,6 +70,7 @@ impl<T, U, Op, DB> QueryFragment<DB> for InsertStatement<T, U, Op> where
         try!(self.operator.collect_binds(out));
         try!(self.target.from_clause().collect_binds(out));
         try!(values.values_bind_params(out));
+        try!(self.returning.collect_binds(out));
         Ok(())
     }
 
@@ -73,25 +79,30 @@ impl<T, U, Op, DB> QueryFragment<DB> for InsertStatement<T, U, Op> where
     }
 }
 
-impl_query_id!(noop: InsertStatement<T, U, Op>);
+impl_query_id!(noop: InsertStatement<T, U, Op, Ret>);
 
-impl<T, U, Op> AsQuery for InsertStatement<T, U, Op> where
+impl<T, U, Op> AsQuery for InsertStatement<T, U, Op, NoReturningClause> where
     T: Table,
-    InsertQuery<T::AllColumns, InsertStatement<T, U, Op>>: Query,
+    InsertStatement<T, U, Op, ReturningClause<T::AllColumns>>: Query,
 {
     type SqlType = <Self::Query as Query>::SqlType;
-    type Query = InsertQuery<T::AllColumns, Self>;
+    type Query = InsertStatement<T, U, Op, ReturningClause<T::AllColumns>>;
 
     fn as_query(self) -> Self::Query {
-        InsertQuery {
-            returning: T::all_columns(),
-            statement: self,
-        }
+        self.returning(T::all_columns())
     }
 }
 
-impl<T, U, Op> InsertStatement<T, U, Op> {
+impl<T, U, Op, Ret> Query for InsertStatement<T, U, Op, ReturningClause<Ret>> where
+    Ret: Expression + SelectableExpression<T> + NonAggregate,
+{
+    type SqlType = Ret::SqlType;
+}
+
+impl<T, U, Op> InsertStatement<T, U, Op, NoReturningClause> {
     /// Specify what expression is returned after execution of the `insert`.
+    /// This method can only be called once.
+    ///
     /// # Examples
     ///
     /// ### Inserting a record:
@@ -124,55 +135,19 @@ impl<T, U, Op> InsertStatement<T, U, Op> {
     /// # #[cfg(not(feature = "postgres"))]
     /// # fn main() {}
     /// ```
-    pub fn returning<E>(self, returns: E) -> InsertQuery<E, Self> where
-        E: Expression + SelectableExpression<T>,
-        InsertQuery<E, Self>: Query,
+    pub fn returning<E>(self, returns: E)
+        -> InsertStatement<T, U, Op, ReturningClause<E>> where
+            InsertStatement<T, U, Op, ReturningClause<E>>: Query,
     {
-        InsertQuery {
-            returning: returns,
-            statement: self,
+        InsertStatement {
+            operator: self.operator,
+            target: self.target,
+            records: self.records,
+            returning: ReturningClause(returns),
         }
     }
 }
 
-#[doc(hidden)]
-#[derive(Debug, Copy, Clone)]
-pub struct InsertQuery<T, U> {
-    returning: T,
-    statement: U,
-}
-
-impl<T, U> Query for InsertQuery<T, U> where
-    T: Expression + NonAggregate,
-{
-    type SqlType = T::SqlType;
-}
-
-impl<T, U, DB> QueryFragment<DB> for InsertQuery<T, U> where
-    DB: Backend + SupportsReturningClause,
-    T: QueryFragment<DB>,
-    U: QueryFragment<DB>,
-{
-    fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
-        try!(self.statement.to_sql(out));
-        out.push_sql(" RETURNING ");
-        try!(self.returning.to_sql(out));
-        Ok(())
-    }
-
-    fn collect_binds(&self, out: &mut DB::BindCollector) -> QueryResult<()> {
-        try!(self.statement.collect_binds(out));
-        try!(self.returning.collect_binds(out));
-        Ok(())
-    }
-
-    fn is_safe_to_cache_prepared(&self) -> bool {
-        false
-    }
-}
-
-impl_query_id!(noop: InsertQuery<T, U>);
-
 #[derive(Debug, Copy, Clone)]
 pub struct Insert;
 

diesel/src/query_builder/mod.rs

@@ -17,6 +17,7 @@ mod group_by_clause;
 mod limit_clause;
 mod offset_clause;
 mod order_clause;
+mod returning_clause;
 mod select_statement;
 pub mod where_clause;
 pub mod insert_statement;

diesel/src/query_builder/returning_clause.rs

@@ -0,0 +1,3 @@
+use backend::SupportsReturningClause;
+
+simple_clause!(NoReturningClause, ReturningClause, " RETURNING ", backend_bounds = SupportsReturningClause);

diesel_compile_tests/tests/compile-fail/insert_statement_does_not_support_returning_methods_on_sqlite.rs

@@ -51,6 +51,6 @@ fn main() {
     insert(&NewUser("Hello".into()))
         .into(users::table)
         .returning(users::name)
-        .get_result(&connection);
+        .get_result::<String>(&connection);
     //~^ ERROR: SupportsReturningClause
 }

diesel_compile_tests/tests/compile-fail/returning_cannot_be_called_twice.rs

@@ -0,0 +1,41 @@
+#[macro_use] extern crate diesel;
+
+use diesel::*;
+use diesel::pg::PgConnection;
+
+table! {
+    users {
+        id -> Integer,
+        name -> VarChar,
+    }
+}
+
+pub struct NewUser(String);
+
+Insertable! {
+    (users)
+    pub struct NewUser(#[column_name(name)] String,);
+}
+
+fn main() {
+    use self::users::dsl::*;
+
+    let connection = PgConnection::establish("").unwrap();
+
+    let query = delete(users.filter(name.eq("Bill")))
+        .returning(id);
+    query.returning(name);
+    //~^ ERROR: no method named `returning`
+
+    let query = insert(&NewUser("Hello".into()))
+        .into(users)
+        .returning(id);
+    query.returning(name);
+    //~^ ERROR: no method named `returning`
+
+    let query = update(users)
+        .set(name.eq("Bill"))
+        .returning(id);
+    query.returning(name);
+    //~^ ERROR: no method named `returning`
+}