Merge pull request #172 from jgallagher/bugfix-insert-multiple-tables

Remove sanity check in `insert()` that could return `StatementFailedToInsertRow`.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rusqlite"
-version = "0.7.2"
+version = "0.7.3"
 authors = ["John Gallagher <jgallagher@bignerdranch.com>"]
 description = "Ergonomic wrapper for SQLite"
 repository = "https://github.com/jgallagher/rusqlite"

Changelog.md

@@ -1,3 +1,9 @@
+# Version 0.7.3 (2016-06-01)
+
+* Fixes an incorrect failure from the `insert()` convenience function when back-to-back inserts to
+  different tables both returned the same row ID
+  ([#171](https://github.com/jgallagher/rusqlite/issues/171)).
+
 # Version 0.7.2 (2016-05-19)
 
 * BREAKING CHANGE: `Rows` no longer implements `Iterator`. It still has a `next()` method, but

src/convenient.rs

@@ -4,18 +4,20 @@ use types::ToSql;
 impl<'conn> Statement<'conn> {
     /// Execute an INSERT and return the ROWID.
     ///
+    /// # Note
+    ///
+    /// This function is a convenience wrapper around `execute()` intended for queries that
+    /// insert a single item. It is possible to misuse this function in a way that it cannot
+    /// detect, such as by calling it on a statement which _updates_ a single item rather than
+    /// inserting one. Please don't do that.
+    ///
     /// # Failure
+    ///
     /// Will return `Err` if no row is inserted or many rows are inserted.
     pub fn insert(&mut self, params: &[&ToSql]) -> Result<i64> {
-        // Some non-insertion queries could still return 1 change (an UPDATE, for example), so
-        // to guard against that we can check that the connection's last_insert_rowid() changes
-        // after we execute the statement.
-        let prev_rowid = self.conn.last_insert_rowid();
         let changes = try!(self.execute(params));
-        let new_rowid = self.conn.last_insert_rowid();
         match changes {
-            1 if prev_rowid != new_rowid => Ok(new_rowid),
-            1 if prev_rowid == new_rowid => Err(Error::StatementFailedToInsertRow),
+            1 => Ok(self.conn.last_insert_rowid()),
             _ => Err(Error::StatementChangedRows(changes)),
         }
     }
@@ -57,18 +59,19 @@ mod test {
     }
 
     #[test]
-    fn test_insert_failures() {
+    fn test_insert_different_tables() {
+        // Test for https://github.com/jgallagher/rusqlite/issues/171
         let db = Connection::open_in_memory().unwrap();
-        db.execute_batch("CREATE TABLE foo(x INTEGER UNIQUE)").unwrap();
-        let mut insert = db.prepare("INSERT INTO foo (x) VALUES (?)").unwrap();
-        let mut update = db.prepare("UPDATE foo SET x = ?").unwrap();
+        db.execute_batch(r"
+            CREATE TABLE foo(x INTEGER);
+            CREATE TABLE bar(x INTEGER);
+        ")
+            .unwrap();
 
-        assert_eq!(insert.insert(&[&1i32]).unwrap(), 1);
-
-        match update.insert(&[&2i32]) {
-            Err(Error::StatementFailedToInsertRow) => (),
-            r => panic!("Unexpected result {:?}", r),
-        }
+        assert_eq!(db.prepare("INSERT INTO foo VALUES (10)").unwrap().insert(&[]).unwrap(),
+                   1);
+        assert_eq!(db.prepare("INSERT INTO bar VALUES (10)").unwrap().insert(&[]).unwrap(),
+                   1);
     }
 
     #[test]

src/error.rs

@@ -55,10 +55,6 @@ pub enum Error {
     /// Error when a query that was expected to insert one row did not insert any or insert many.
     StatementChangedRows(c_int),
 
-    /// Error when a query that was expected to insert a row did not change the connection's
-    /// last_insert_rowid().
-    StatementFailedToInsertRow,
-
     /// Error returned by `functions::Context::get` when the function argument cannot be converted
     /// to the requested type.
     #[cfg(feature = "functions")]
@@ -105,7 +101,6 @@ impl fmt::Display for Error {
             Error::InvalidColumnName(ref name) => write!(f, "Invalid column name: {}", name),
             Error::InvalidColumnType => write!(f, "Invalid column type"),
             Error::StatementChangedRows(i) => write!(f, "Query changed {} rows", i),
-            Error::StatementFailedToInsertRow => write!(f, "Statement failed to insert new row"),
 
             #[cfg(feature = "functions")]
             Error::InvalidFunctionParameterType => write!(f, "Invalid function parameter type"),
@@ -136,7 +131,6 @@ impl error::Error for Error {
             Error::InvalidColumnName(_) => "invalid column name",
             Error::InvalidColumnType => "invalid column type",
             Error::StatementChangedRows(_) => "query inserted zero or more than one row",
-            Error::StatementFailedToInsertRow => "statement failed to insert new row",
 
             #[cfg(feature = "functions")]
             Error::InvalidFunctionParameterType => "invalid function parameter type",
@@ -161,8 +155,7 @@ impl error::Error for Error {
             Error::InvalidColumnName(_) |
             Error::InvalidColumnType |
             Error::InvalidPath(_) |
-            Error::StatementChangedRows(_) |
-            Error::StatementFailedToInsertRow => None,
+            Error::StatementChangedRows(_) => None,
 
             #[cfg(feature = "functions")]
             Error::InvalidFunctionParameterType => None,