Skip to content

Commit

Permalink
Remove scary lifetime-of-rows-may-panic from README.
Browse files Browse the repository at this point in the history
Closes #119.
  • Loading branch information
hydhknn committed Feb 1, 2016
1 parent 4149389 commit a793f8c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 46 deletions.
39 changes: 0 additions & 39 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,45 +73,6 @@ features](http://doc.crates.io/manifest.html#the-features-section). They are:
* [`blob`](http://jgallagher.github.io/rusqlite/rusqlite/blob/index.html)
gives `std::io::{Read, Write, Seek}` access to SQL BLOBs.

### Design of Rows and Row

To retrieve the result rows from a query, SQLite requires you to call
[sqlite3_step()](https://www.sqlite.org/c3ref/step.html) on a prepared statement. You can only
retrieve the values of the "current" row. From the Rust point of view, this means that each row
is only valid until the next row is fetched. [rust-sqlite3](https://github.com/dckc/rust-sqlite3)
solves this the correct way with lifetimes. However, this means that the result rows do not
satisfy the [Iterator](http://doc.rust-lang.org/std/iter/trait.Iterator.html) trait, which means
you cannot (as easily) loop over the rows, or use many of the helpful Iterator methods like `map`
and `filter`.

Instead, Rusqlite's `Rows` handle does conform to `Iterator`. It ensures safety by
performing checks at runtime to ensure you do not try to retrieve the values of a "stale" row, and
will panic if you do so. A specific example that will panic:

```rust
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 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)
}
```

There are other, less obvious things that may result in a panic as well, such as calling
`collect()` on a `Rows` and then trying to use the collected rows.

Strongly consider using the method `query_map()` instead, if you can.
`query_map()` returns an iterator over rows-mapped-to-some-type. This
iterator does not have any of the above issues with panics due to attempting to
access stale rows.

## Author

John Gallagher, johnkgallagher@gmail.com
Expand Down
11 changes: 4 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,9 @@ pub type SqliteRows<'stmt> = Rows<'stmt>;
///
/// ## 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:
Expand All @@ -994,7 +997,7 @@ pub type SqliteRows<'stmt> = Rows<'stmt>;
/// let mut rows = try!(stmt.query(&[]));
///
/// let row0 = try!(rows.next().unwrap());
/// // row 0 is value now...
/// // row 0 is valid for now...
///
/// let row1 = try!(rows.next().unwrap());
/// // row 0 is now STALE, and row 1 is valid
Expand All @@ -1008,12 +1011,6 @@ pub type SqliteRows<'stmt> = Rows<'stmt>;
/// (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).
///
/// This problem could be solved by changing the signature of `next` to tie the lifetime of the
/// returned row to the lifetime of (a mutable reference to) the result rows handle, but this would
/// no longer implement `Iterator`, and therefore you would lose access to the majority of
/// functions which are useful (such as support for `for ... in ...` looping, `map`, `filter`,
/// etc.).
pub struct Rows<'stmt> {
stmt: &'stmt Statement<'stmt>,
current_row: Rc<Cell<c_int>>,
Expand Down

0 comments on commit a793f8c

Please sign in to comment.