From a793f8c8c57edbc23f69fdd0b171f3fea2d99996 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 1 Feb 2016 14:30:51 -0500 Subject: [PATCH 1/3] Remove scary lifetime-of-rows-may-panic from README. Closes #119. --- README.md | 39 --------------------------------------- src/lib.rs | 11 ++++------- 2 files changed, 4 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index bc56bc3..05635c7 100644 --- a/README.md +++ b/README.md @@ -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 { - 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 diff --git a/src/lib.rs b/src/lib.rs index d4b445e..7a11f64 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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: @@ -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 @@ -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>, From 350fd11fed963b73f8e4b0ec206c57e0f4c636fe Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 1 Feb 2016 15:21:03 -0500 Subject: [PATCH 2/3] Add a `handle()` method to unsafely get the underlying SQLite connection. Doc comments suggest opening issues on rusqlite for any uses of `handle()`, as uses indicate areas where rusqlite insufficiently wraps SQLite. --- src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 7a11f64..6b52a95 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -494,6 +494,18 @@ impl Connection { self.db.borrow_mut().load_extension(dylib_path.as_ref(), entry_point) } + /// Get access to the underlying SQLite database connection handle. + /// + /// # Warning + /// + /// You should not need to use this function. If you do need to, please [open an issue + /// on the rusqlite repository](https://github.com/jgallagher/rusqlite/issues) and describe + /// your use case. This function is unsafe because it gives you raw access to the SQLite + /// connection, and what you do with it could impact the safety of this `Connection`. + pub unsafe fn handle(&self) -> *mut ffi::Struct_sqlite3 { + self.db.borrow().db() + } + fn decode_result(&self, code: c_int) -> Result<()> { self.db.borrow_mut().decode_result(code) } From d6d9fa9f6324500e765cff9ca28047577406f732 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 1 Feb 2016 15:23:05 -0500 Subject: [PATCH 3/3] Add handle() to Changelog --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 40c1642..63828da 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ * Introduces `ZeroBlob` type under the `blob` module/feature exposing SQLite's zeroblob API. * Adds CI testing for Windows via AppVeyor. * Fixes a warning building libsqlite3-sys under Rust 1.6. +* Adds an unsafe `handle()` method to `Connection`. Please file an issue if you actually use it. # Version 0.6.0 (2015-12-17)