Merge pull request #153 from jgallagher/remove-rows-iterator-impl

Remove Rows's implementation of Iterator
This commit is contained in:
John Gallagher 2016-05-19 14:31:07 -05:00
commit c6204da9b7
5 changed files with 250 additions and 150 deletions

View File

@ -1,5 +1,15 @@
# Version UPCOMING (...)
* BREAKING CHANGE: `Rows` no longer implements `Iterator`. It still has a `next()` method, but
the lifetime of the returned `Row` is now tied to the lifetime of the vending `Rows` object.
This behavior is more correct. Previously there were runtime checks to prevent misuse, but
other changes in this release to reset statements as soon as possible introduced yet another
hazard related to the lack of these lifetime connections. We were already recommending the
use of `query_map` and `query_and_then` over raw `query`; both of theose still return handles
that implement `Iterator`.
* BREAKING CHANGE: `Transaction::savepoint()` now returns a `Savepoint` instead of another
`Transaction`. Unlike `Transaction`, `Savepoint`s can be rolled back while keeping the current
savepoint active.
* BREAKING CHANGE: Creating transactions from a `Connection` or savepoints from a `Transaction`
now take `&mut self` instead of `&self` to correctly represent that transactions within a
connection are inherently nested. While a transaction is alive, the parent connection or
@ -7,13 +17,11 @@
access to `Connection`'s methods via the `Transaction` itself.
* BREAKING CHANGE: `Transaction::set_commit` and `Transaction::set_rollback` have been replaced
by `Transaction::set_drop_behavior`.
* BREAKING CHANGE: `Transaction::savepoint()` now returns a `Savepoint` instead of another
`Transaction`. Unlike `Transaction`, `Savepoint`s can be rolled back while keeping the current
savepoint active.
* Adds `Connection::prepare_cached`. `Connection` now keeps an internal cache of any statements
prepared via this method. The size of this cache defaults to 16 (`prepare_cached` will always
work but may re-prepare statements if more are prepared than the cache holds), and can be
controlled via `Connection::set_prepared_statement_cache_capacity`.
* Adds `query_map_named` and `query_and_then_named` to `Statement`.
* Adds `insert` convenience method to `Statement` which returns the row ID of an inserted row.
* Adds `exists` convenience method returning whether a query finds one or more rows.
* Adds support for serializing types from the `serde_json` crate. Requires the `serde_json` feature.

View File

@ -40,10 +40,6 @@ pub enum Error {
/// did not return any.
QueryReturnedNoRows,
/// Error when trying to access a `Row` after stepping past it. See the discussion on
/// the `Rows` type for more details.
GetFromStaleRow,
/// Error when the value of a particular column is requested, but the index is out of range
/// for the statement.
InvalidColumnIndex(c_int),
@ -105,7 +101,6 @@ impl fmt::Display for Error {
write!(f, "Execute returned results - did you mean to call query?")
}
Error::QueryReturnedNoRows => write!(f, "Query returned no rows"),
Error::GetFromStaleRow => write!(f, "Attempted to get a value from a stale row"),
Error::InvalidColumnIndex(i) => write!(f, "Invalid column index: {}", i),
Error::InvalidColumnName(ref name) => write!(f, "Invalid column name: {}", name),
Error::InvalidColumnType => write!(f, "Invalid column type"),
@ -137,7 +132,6 @@ impl error::Error for Error {
"execute returned results - did you mean to call query?"
}
Error::QueryReturnedNoRows => "query returned no rows",
Error::GetFromStaleRow => "attempted to get a value from a stale row",
Error::InvalidColumnIndex(_) => "invalid column index",
Error::InvalidColumnName(_) => "invalid column name",
Error::InvalidColumnType => "invalid column type",
@ -163,7 +157,6 @@ impl error::Error for Error {
Error::InvalidParameterName(_) |
Error::ExecuteReturnedResults |
Error::QueryReturnedNoRows |
Error::GetFromStaleRow |
Error::InvalidColumnIndex(_) |
Error::InvalidColumnName(_) |
Error::InvalidColumnType |

View File

@ -64,12 +64,12 @@ extern crate lazy_static;
use std::default::Default;
use std::convert;
use std::marker::PhantomData;
use std::mem;
use std::ptr;
use std::fmt;
use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::cell::{RefCell, Cell};
use std::cell::RefCell;
use std::ffi::{CStr, CString};
use std::result;
use std::str;
@ -768,7 +768,11 @@ impl<'conn> Statement<'conn> {
}
}
/// Execute the prepared statement, returning an iterator over the resulting rows.
/// Execute the prepared statement, returning a handle to the resulting rows.
///
/// Due to lifetime restricts, the rows handle returned by `query` does not
/// implement the `Iterator` trait. Consider using `query_map` or `query_and_then`
/// instead, which do.
///
/// ## Example
///
@ -779,7 +783,7 @@ impl<'conn> Statement<'conn> {
/// let mut rows = try!(stmt.query(&[]));
///
/// let mut names = Vec::new();
/// for result_row in rows {
/// while let Some(result_row) = rows.next() {
/// let row = try!(result_row);
/// names.push(row.get(0));
/// }
@ -788,7 +792,7 @@ impl<'conn> Statement<'conn> {
/// }
/// ```
///
/// # Failure
/// ## Failure
///
/// Will return `Err` if binding parameters fails.
pub fn query<'a>(&'a mut self, params: &[&ToSql]) -> Result<Rows<'a>> {
@ -796,13 +800,27 @@ impl<'conn> Statement<'conn> {
Ok(Rows::new(self))
}
/// Executes the prepared statement and maps a function over the resulting
/// rows.
/// Executes the prepared statement and maps a function over the resulting rows, returning
/// an iterator over the mapped function results.
///
/// Unlike the iterator produced by `query`, the returned iterator does not expose the possibility
/// for accessing stale rows.
/// ## Example
///
/// # Failure
/// ```rust,no_run
/// # use rusqlite::{Connection, Result};
/// fn get_names(conn: &Connection) -> Result<Vec<String>> {
/// let mut stmt = try!(conn.prepare("SELECT name FROM people"));
/// let rows = try!(stmt.query_map(&[], |row| row.get(0)));
///
/// let mut names = Vec::new();
/// for name_result in rows {
/// names.push(try!(name_result));
/// }
///
/// Ok(names)
/// }
/// ```
///
/// ## Failure
///
/// Will return `Err` if binding parameters fails.
pub fn query_map<'a, T, F>(&'a mut self, params: &[&ToSql], f: F) -> Result<MappedRows<'a, F>>
@ -820,9 +838,6 @@ impl<'conn> Statement<'conn> {
/// rows, where the function returns a `Result` with `Error` type implementing
/// `std::convert::From<Error>` (so errors can be unified).
///
/// Unlike the iterator produced by `query`, the returned iterator does not expose the possibility
/// for accessing stale rows.
///
/// # Failure
///
/// Will return `Err` if binding parameters fails.
@ -913,7 +928,8 @@ impl<'stmt, T, F> Iterator for MappedRows<'stmt, F>
type Item = Result<T>;
fn next(&mut self) -> Option<Result<T>> {
self.rows.next().map(|row_result| row_result.map(|row| (self.map)(&row)))
let map = &mut self.map;
self.rows.next().map(|row_result| row_result.map(|row| (map)(&row)))
}
}
@ -931,9 +947,10 @@ impl<'stmt, T, E, F> Iterator for AndThenRows<'stmt, F>
type Item = result::Result<T, E>;
fn next(&mut self) -> Option<Self::Item> {
let map = &mut self.map;
self.rows.next().map(|row_result| {
row_result.map_err(E::from)
.and_then(|row| (self.map)(&row))
.and_then(|row| (map)(&row))
})
}
}
@ -941,52 +958,19 @@ impl<'stmt, T, E, F> Iterator for AndThenRows<'stmt, F>
/// Old name for `Rows`. `SqliteRows` is deprecated.
pub type SqliteRows<'stmt> = Rows<'stmt>;
/// An iterator over the resulting rows of a query.
///
/// ## Warning
///
/// Strongly consider using `query_map` or `query_and_then` instead of `query`; the former do not
/// suffer from the following problem.
///
/// Due to the way SQLite returns result rows of a query, it is not safe to attempt to get values
/// from a row after it has become stale (i.e., `next()` has been called again on the `Rows`
/// iterator). For example:
///
/// ```rust,no_run
/// # use rusqlite::{Connection, Result};
/// fn bad_function_will_panic(conn: &Connection) -> Result<i64> {
/// let mut stmt = try!(conn.prepare("SELECT id FROM my_table"));
/// let mut rows = try!(stmt.query(&[]));
///
/// let row0 = try!(rows.next().unwrap());
/// // row 0 is valid for now...
///
/// let row1 = try!(rows.next().unwrap());
/// // row 0 is now STALE, and row 1 is valid
///
/// let my_id = row0.get(0); // WILL PANIC because row 0 is stale
/// Ok(my_id)
/// }
/// ```
///
/// Please note that this means some of the methods on `Iterator` are not useful, such as `collect`
/// (which would result in a collection of rows, only the last of which can safely be used) and
/// `min`/`max` (which could return a stale row unless the last row happened to be the min or max,
/// respectively).
/// An handle for the resulting rows of a query.
pub struct Rows<'stmt> {
stmt: Option<&'stmt Statement<'stmt>>,
current_row: Rc<Cell<c_int>>,
}
impl<'stmt> Rows<'stmt> {
fn new(stmt: &'stmt Statement<'stmt>) -> Rows<'stmt> {
Rows {
stmt: Some(stmt),
current_row: Rc::new(Cell::new(0)),
}
}
fn get_expected_row(&mut self) -> Result<Row<'stmt>> {
fn get_expected_row<'a>(&'a mut self) -> Result<Row<'a, 'stmt>> {
match self.next() {
Some(row) => row,
None => Err(Error::QueryReturnedNoRows),
@ -998,21 +982,24 @@ impl<'stmt> Rows<'stmt> {
stmt.stmt.reset();
}
}
}
impl<'stmt> Iterator for Rows<'stmt> {
type Item = Result<Row<'stmt>>;
fn next(&mut self) -> Option<Result<Row<'stmt>>> {
/// Attempt to get the next row from the query. Returns `Some(Ok(Row))` if there
/// is another row, `Some(Err(...))` if there was an error getting the next
/// row, and `None` if all rows have been retrieved.
///
/// ## Note
///
/// This interface is not compatible with Rust's `Iterator` trait, because the
/// lifetime of the returned row is tied to the lifetime of `self`. This is a
/// "streaming iterator". For a more natural interface, consider using `query_map`
/// or `query_and_then` instead, which return types that implement `Iterator`.
pub fn next<'a>(&'a mut self) -> Option<Result<Row<'a, 'stmt>>> {
self.stmt.and_then(|stmt| {
match stmt.stmt.step() {
ffi::SQLITE_ROW => {
let current_row = self.current_row.get() + 1;
self.current_row.set(current_row);
Some(Ok(Row {
stmt: stmt,
current_row: self.current_row.clone(),
row_idx: current_row,
phantom: PhantomData,
}))
}
ffi::SQLITE_DONE => {
@ -1035,46 +1022,22 @@ impl<'stmt> Drop for Rows<'stmt> {
}
/// Old name for `Row`. `SqliteRow` is deprecated.
pub type SqliteRow<'stmt> = Row<'stmt>;
pub type SqliteRow<'a, 'stmt> = Row<'a, 'stmt>;
/// A single result row of a query.
pub struct Row<'stmt> {
pub struct Row<'a, 'stmt> {
stmt: &'stmt Statement<'stmt>,
current_row: Rc<Cell<c_int>>,
row_idx: c_int,
phantom: PhantomData<&'a ()>,
}
impl<'stmt> Row<'stmt> {
impl<'a, 'stmt> Row<'a, 'stmt> {
/// Get the value of a particular column of the result row.
///
/// Note that `Row` can panic at runtime if you use it incorrectly. When you are
/// retrieving the rows of a query, a row becomes stale once you have requested the next row,
/// and the values can no longer be retrieved. In general (when using looping over the rows,
/// for example) this isn't an issue, but it means you cannot do something like this:
///
/// ```rust,no_run
/// # use rusqlite::{Connection, Result};
/// fn bad_function_will_panic(conn: &Connection) -> Result<i64> {
/// let mut stmt = try!(conn.prepare("SELECT id FROM my_table"));
/// let mut rows = try!(stmt.query(&[]));
///
/// let row0 = try!(rows.next().unwrap());
/// // row 0 is value now...
///
/// let row1 = try!(rows.next().unwrap());
/// // row 0 is now STALE, and row 1 is valid
///
/// let my_id = row0.get(0); // WILL PANIC because row 0 is stale
/// Ok(my_id)
/// }
/// ```
///
/// ## Failure
///
/// Panics if the underlying SQLite column type is not a valid type as a source for `T`.
///
/// Panics if `idx` is outside the range of columns in the returned query or if this row
/// is stale.
/// Panics if `idx` is outside the range of columns in the returned query.
pub fn get<I: RowIndex, T: FromSql>(&self, idx: I) -> T {
self.get_checked(idx).unwrap()
}
@ -1091,12 +1054,7 @@ impl<'stmt> Row<'stmt> {
///
/// Returns an `Error::InvalidColumnName` if `idx` is not a valid column name
/// for this row.
///
/// Returns an `Error::GetFromStaleRow` if this row is stale.
pub fn get_checked<I: RowIndex, T: FromSql>(&self, idx: I) -> Result<T> {
if self.row_idx != self.current_row.get() {
return Err(Error::GetFromStaleRow);
}
unsafe {
let idx = try!(idx.idx(self.stmt));
@ -1287,15 +1245,24 @@ mod test {
let mut query = db.prepare("SELECT x FROM foo WHERE x < ? ORDER BY x DESC").unwrap();
{
let rows = query.query(&[&4i32]).unwrap();
let v: Vec<i32> = rows.map(|r| r.unwrap().get(0)).collect();
let mut rows = query.query(&[&4i32]).unwrap();
let mut v = Vec::<i32>::new();
while let Some(row) = rows.next() {
v.push(row.unwrap().get(0));
}
assert_eq!(v, [3i32, 2, 1]);
}
{
let rows = query.query(&[&3i32]).unwrap();
let v: Vec<i32> = rows.map(|r| r.unwrap().get(0)).collect();
let mut rows = query.query(&[&3i32]).unwrap();
let mut v = Vec::<i32>::new();
while let Some(row) = rows.next() {
v.push(row.unwrap().get(0));
}
assert_eq!(v, [2i32, 1]);
}
}
@ -1358,26 +1325,6 @@ mod test {
assert!(format!("{}", err).contains("does_not_exist"));
}
#[test]
fn test_row_expiration() {
let db = checked_memory_handle();
db.execute_batch("CREATE TABLE foo(x INTEGER)").unwrap();
db.execute_batch("INSERT INTO foo(x) VALUES(1)").unwrap();
db.execute_batch("INSERT INTO foo(x) VALUES(2)").unwrap();
let mut stmt = db.prepare("SELECT x FROM foo ORDER BY x").unwrap();
let mut rows = stmt.query(&[]).unwrap();
let first = rows.next().unwrap().unwrap();
let second = rows.next().unwrap().unwrap();
assert_eq!(2i32, second.get(0));
match first.get_checked::<i32, i32>(0).unwrap_err() {
Error::GetFromStaleRow => (),
err => panic!("Unexpected error {}", err),
}
}
#[test]
fn test_last_insert_rowid() {
let db = checked_memory_handle();

View File

@ -1,6 +1,8 @@
use std::convert;
use std::result;
use libc::c_int;
use {Result, Error, Connection, Statement, Rows, Row, str_to_cstring};
use {Result, Error, Connection, Statement, MappedRows, AndThenRows, Rows, Row, str_to_cstring};
use types::ToSql;
impl Connection {
@ -84,7 +86,7 @@ impl<'conn> Statement<'conn> {
self.execute_()
}
/// Execute the prepared statement with named parameter(s), returning an iterator over the
/// Execute the prepared statement with named parameter(s), returning a handle for the
/// resulting rows. If any parameters that were in the prepared statement are not included in
/// `params`, they will continue to use the most-recently bound value from a previous call to
/// `query_named`, or `NULL` if they have never been bound.
@ -96,7 +98,7 @@ impl<'conn> Statement<'conn> {
/// fn query(conn: &Connection) -> Result<()> {
/// let mut stmt = try!(conn.prepare("SELECT * FROM test where name = :name"));
/// let mut rows = try!(stmt.query_named(&[(":name", &"one")]));
/// for row in rows {
/// while let Some(row) = rows.next() {
/// // ...
/// }
/// Ok(())
@ -111,6 +113,94 @@ impl<'conn> Statement<'conn> {
Ok(Rows::new(self))
}
/// Execute the prepared statement with named parameter(s), returning an iterator over the
/// result of calling the mapping function over the query's rows. If any parameters that were
/// in the prepared statement are not included in `params`, they will continue to use the
/// most-recently bound value from a previous call to `query_named`, or `NULL` if they have
/// never been bound.
///
/// ## Example
///
/// ```rust,no_run
/// # use rusqlite::{Connection, Result};
/// fn get_names(conn: &Connection) -> Result<Vec<String>> {
/// let mut stmt = try!(conn.prepare("SELECT name FROM people WHERE id = :id"));
/// let rows = try!(stmt.query_map_named(&[(":id", &"one")], |row| row.get(0)));
///
/// let mut names = Vec::new();
/// for name_result in rows {
/// names.push(try!(name_result));
/// }
///
/// Ok(names)
/// }
/// ```
///
/// ## Failure
///
/// Will return `Err` if binding parameters fails.
pub fn query_map_named<'a, T, F>(&'a mut self,
params: &[(&str, &ToSql)],
f: F)
-> Result<MappedRows<'a, F>>
where F: FnMut(&Row) -> T
{
let rows = try!(self.query_named(params));
Ok(MappedRows {
rows: rows,
map: f,
})
}
/// Execute the prepared statement with named parameter(s), returning an iterator over the
/// result of calling the mapping function over the query's rows. If any parameters that were
/// in the prepared statement are not included in `params`, they will continue to use the
/// most-recently bound value from a previous call to `query_named`, or `NULL` if they have
/// never been bound.
///
/// ## Example
///
/// ```rust,no_run
/// # use rusqlite::{Connection, Result};
/// struct Person { name: String };
///
/// fn name_to_person(name: String) -> Result<Person> {
/// // ... check for valid name
/// Ok(Person{ name: name })
/// }
///
/// fn get_names(conn: &Connection) -> Result<Vec<Person>> {
/// let mut stmt = try!(conn.prepare("SELECT name FROM people WHERE id = :id"));
/// let rows = try!(stmt.query_and_then_named(&[(":id", &"one")], |row| {
/// name_to_person(row.get(0))
/// }));
///
/// let mut persons = Vec::new();
/// for person_result in rows {
/// persons.push(try!(person_result));
/// }
///
/// Ok(persons)
/// }
/// ```
///
/// ## Failure
///
/// Will return `Err` if binding parameters fails.
pub fn query_and_then_named<'a, T, E, F>(&'a mut self,
params: &[(&str, &ToSql)],
f: F)
-> Result<AndThenRows<'a, F>>
where E: convert::From<Error>,
F: FnMut(&Row) -> result::Result<T, E>
{
let rows = try!(self.query_named(params));
Ok(AndThenRows {
rows: rows,
map: f,
})
}
fn bind_parameters_named(&mut self, params: &[(&str, &ToSql)]) -> Result<()> {
for &(name, value) in params {
if let Some(i) = try!(self.parameter_index(name)) {
@ -126,6 +216,7 @@ impl<'conn> Statement<'conn> {
#[cfg(test)]
mod test {
use Connection;
use error::Error;
#[test]
fn test_execute_named() {
@ -139,9 +230,9 @@ mod test {
assert_eq!(3i32,
db.query_row_named("SELECT SUM(x) FROM foo WHERE x > :x",
&[(":x", &0i32)],
|r| r.get(0))
.unwrap());
&[(":x", &0i32)],
|r| r.get(0))
.unwrap());
}
#[test]
@ -156,20 +247,77 @@ mod test {
assert_eq!(1i32,
db.query_row_named("SELECT COUNT(*) FROM test WHERE name = :name",
&[(":name", &"one")],
|r| r.get(0))
.unwrap());
&[(":name", &"one")],
|r| r.get(0))
.unwrap());
}
#[test]
fn test_query_named() {
let db = Connection::open_in_memory().unwrap();
let sql = "CREATE TABLE test (id INTEGER PRIMARY KEY NOT NULL, name TEXT NOT NULL, flag \
INTEGER)";
let sql = r#"
CREATE TABLE test (id INTEGER PRIMARY KEY NOT NULL, name TEXT NOT NULL, flag INTEGER);
INSERT INTO test(id, name) VALUES (1, "one");
"#;
db.execute_batch(sql).unwrap();
let mut stmt = db.prepare("SELECT * FROM test where name = :name").unwrap();
stmt.query_named(&[(":name", &"one")]).unwrap();
let mut stmt = db.prepare("SELECT id FROM test where name = :name").unwrap();
let mut rows = stmt.query_named(&[(":name", &"one")]).unwrap();
let id: i32 = rows.next().unwrap().unwrap().get(0);
assert_eq!(1, id);
}
#[test]
fn test_query_map_named() {
let db = Connection::open_in_memory().unwrap();
let sql = r#"
CREATE TABLE test (id INTEGER PRIMARY KEY NOT NULL, name TEXT NOT NULL, flag INTEGER);
INSERT INTO test(id, name) VALUES (1, "one");
"#;
db.execute_batch(sql).unwrap();
let mut stmt = db.prepare("SELECT id FROM test where name = :name").unwrap();
let mut rows = stmt.query_map_named(&[(":name", &"one")], |row| {
let id: i32 = row.get(0);
2 * id
}).unwrap();
let doubled_id: i32 = rows.next().unwrap().unwrap();
assert_eq!(2, doubled_id);
}
#[test]
fn test_query_and_then_named() {
let db = Connection::open_in_memory().unwrap();
let sql = r#"
CREATE TABLE test (id INTEGER PRIMARY KEY NOT NULL, name TEXT NOT NULL, flag INTEGER);
INSERT INTO test(id, name) VALUES (1, "one");
INSERT INTO test(id, name) VALUES (2, "one");
"#;
db.execute_batch(sql).unwrap();
let mut stmt = db.prepare("SELECT id FROM test where name = :name ORDER BY id ASC").unwrap();
let mut rows = stmt.query_and_then_named(&[(":name", &"one")], |row| {
let id: i32 = row.get(0);
if id == 1 {
Ok(id)
} else {
Err(Error::SqliteSingleThreadedMode)
}
}).unwrap();
// first row should be Ok
let doubled_id: i32 = rows.next().unwrap().unwrap();
assert_eq!(1, doubled_id);
// second row should be Err
match rows.next().unwrap() {
Ok(_) => panic!("invalid Ok"),
Err(Error::SqliteSingleThreadedMode) => (),
Err(_) => panic!("invalid Err"),
}
}
#[test]

View File

@ -353,17 +353,21 @@ mod test {
let mut stmt = db.prepare("SELECT t, b FROM foo ORDER BY ROWID ASC").unwrap();
let mut rows = stmt.query(&[]).unwrap();
let row1 = rows.next().unwrap().unwrap();
let s1: Option<String> = row1.get(0);
let b1: Option<Vec<u8>> = row1.get(1);
assert_eq!(s.unwrap(), s1.unwrap());
assert!(b1.is_none());
{
let row1 = rows.next().unwrap().unwrap();
let s1: Option<String> = row1.get(0);
let b1: Option<Vec<u8>> = row1.get(1);
assert_eq!(s.unwrap(), s1.unwrap());
assert!(b1.is_none());
}
let row2 = rows.next().unwrap().unwrap();
let s2: Option<String> = row2.get(0);
let b2: Option<Vec<u8>> = row2.get(1);
assert!(s2.is_none());
assert_eq!(b, b2);
{
let row2 = rows.next().unwrap().unwrap();
let s2: Option<String> = row2.get(0);
let b2: Option<Vec<u8>> = row2.get(1);
assert!(s2.is_none());
assert_eq!(b, b2);
}
}
#[test]