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.
This commit is contained in:
John Gallagher 2016-06-01 20:52:22 -04:00
parent 1950158c87
commit ad0b823560
2 changed files with 21 additions and 25 deletions

View File

@ -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]

View File

@ -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,