From ad0b823560852e0939f78a6a4de4e3bda92b2a35 Mon Sep 17 00:00:00 2001
From: John Gallagher <jgallagher@bignerdranch.com>
Date: Wed, 1 Jun 2016 20:52:22 -0400
Subject: [PATCH] Remove sanity check in `insert()` that could return
 `StatementFailedToInsertRow`.

This was intended to detect an `UPDATE` query passed to `insert`, but
incorrectly failed if inserts to different tables caused the same row ID
to be returned from both. `UPDATE`s are no longer detectable.
---
 src/convenient.rs | 37 ++++++++++++++++++++-----------------
 src/error.rs      |  9 +--------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/convenient.rs b/src/convenient.rs
index 5f867b4..22817a5 100644
--- a/src/convenient.rs
+++ b/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]
diff --git a/src/error.rs b/src/error.rs
index 1d6ccd4..f5d37aa 100644
--- a/src/error.rs
+++ b/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,