From 25de884720f5441bae3cff1491a9a57c3140a417 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sun, 2 Aug 2015 12:07:49 +0200 Subject: [PATCH 01/39] LRU statement cache --- Cargo.toml | 5 ++++ src/cache.rs | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 9 +++++++ 3 files changed, 89 insertions(+) create mode 100644 src/cache.rs diff --git a/Cargo.toml b/Cargo.toml index 5d05ed5..1cd05f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ name = "rusqlite" [features] load_extension = ["libsqlite3-sys/load_extension"] +cache = ["lru-cache"] [dependencies] time = "~0.1.0" @@ -26,3 +27,7 @@ tempdir = "~0.3.4" [dependencies.libsqlite3-sys] path = "libsqlite3-sys" version = "0.2.0" + +[dependencies.lru-cache] +version = "~0.0.4" +optional = true \ No newline at end of file diff --git a/src/cache.rs b/src/cache.rs new file mode 100644 index 0000000..2b3165b --- /dev/null +++ b/src/cache.rs @@ -0,0 +1,75 @@ +extern crate lru_cache; + +use {SqliteResult, SqliteConnection, SqliteStatement}; +use self::lru_cache::LruCache; + +/// Prepared statements are cached for faster execution. +/// FIXME limitation: the same SQL can be cached only once... +#[derive(Debug)] +pub struct StatementCache<'conn> { + pub conn: &'conn SqliteConnection, + cache: LruCache>, +} + +impl<'conn> StatementCache<'conn> { + pub fn new(conn: &'conn SqliteConnection, capacity: usize) -> StatementCache<'conn> { + StatementCache{ conn: conn, cache: LruCache::new(capacity) } + } + + /// Search the cache for a prepared-statement object that implements `sql`. + // If no such prepared-statement can be found, allocate and prepare a new one. + pub fn get(&mut self, sql: &str) -> SqliteResult> { + let stmt = self.cache.remove(sql); + match stmt { + Some(stmt) => Ok(stmt), + _ => self.conn.prepare(sql) + } + } + + /// If `discard` is true, then the statement is deleted immediately. + /// Otherwise it is added to the LRU list and may be returned + /// by a subsequent call to `get()`. + pub fn release(&mut self, stmt: SqliteStatement<'conn>, discard: bool) -> SqliteResult<()> { + if discard { + return stmt.finalize(); + } + // FIXME stmt.reset_if_needed(); + // clear bindings ??? + self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) + } + + /// Flush the prepared statement cache + pub fn flush(&mut self) { + self.cache.clear(); + } + + /// Return (current, max) sizes. + pub fn size(&self) -> (usize, usize) { + (self.cache.len(), self.cache.capacity()) + } + + /// Set the maximum number of cached statements. + pub fn set_size(&mut self, capacity: usize) { + self.cache.set_capacity(capacity); + } +} + +#[cfg(test)] +mod test { + use SqliteConnection; + use super::StatementCache; + + #[test] + fn test_cache() { + let db = SqliteConnection::open_in_memory().unwrap(); + let mut cache = StatementCache::new(&db, 10); + let sql = "PRAGMA schema_version"; + let mut stmt = cache.get(sql).unwrap(); + //println!("NEW {:?}", stmt); + cache.release(stmt, false).unwrap(); + stmt = cache.get(sql).unwrap(); + //println!("CACHED {:?}", stmt); + cache.release(stmt, true).unwrap(); + cache.flush(); + } +} diff --git a/src/lib.rs b/src/lib.rs index f98780b..7949b3f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,6 +79,7 @@ pub use transaction::{SqliteTransactionBehavior, pub mod types; mod transaction; #[cfg(feature = "load_extension")] mod load_extension_guard; +#[cfg(feature = "cache")] pub mod cache; /// A typedef of the result returned by many methods. pub type SqliteResult = Result; @@ -710,6 +711,14 @@ impl<'conn> SqliteStatement<'conn> { } } + fn sql(&self) -> String { // TODO Maybe SQL should by kept as an SqliteStatement field ? + unsafe { + let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); + let utf8_str = str::from_utf8(c_slice); + utf8_str.unwrap().to_string() + } + } + fn finalize_(&mut self) -> SqliteResult<()> { let r = unsafe { ffi::sqlite3_finalize(self.stmt) }; self.stmt = ptr::null_mut(); From a470499dcb4b8399072a0e1eece514c705fb29cb Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sat, 8 Aug 2015 16:33:08 +0200 Subject: [PATCH 02/39] Improve documentation --- src/cache.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 2b3165b..8724766 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,9 +1,11 @@ +//! Prepared statements cache for faster execution. extern crate lru_cache; use {SqliteResult, SqliteConnection, SqliteStatement}; use self::lru_cache::LruCache; -/// Prepared statements are cached for faster execution. +/// Prepared statements cache. +/// /// FIXME limitation: the same SQL can be cached only once... #[derive(Debug)] pub struct StatementCache<'conn> { @@ -12,6 +14,7 @@ pub struct StatementCache<'conn> { } impl<'conn> StatementCache<'conn> { + /// Create a statement cache. pub fn new(conn: &'conn SqliteConnection, capacity: usize) -> StatementCache<'conn> { StatementCache{ conn: conn, cache: LruCache::new(capacity) } } From 4d2d8b43e65c6a44e988a5373782cd53626cd740 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 11 Nov 2015 14:45:25 +0100 Subject: [PATCH 03/39] Clean use statement. --- src/cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 8724766..ae03c67 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -2,7 +2,7 @@ extern crate lru_cache; use {SqliteResult, SqliteConnection, SqliteStatement}; -use self::lru_cache::LruCache; +use lru_cache::LruCache; /// Prepared statements cache. /// From fa03bcd5647d017a21abd7cc5d4cf3b085c1dcff Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sat, 5 Dec 2015 12:28:12 +0100 Subject: [PATCH 04/39] Fix use declaration. --- src/cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index ae03c67..8724766 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -2,7 +2,7 @@ extern crate lru_cache; use {SqliteResult, SqliteConnection, SqliteStatement}; -use lru_cache::LruCache; +use self::lru_cache::LruCache; /// Prepared statements cache. /// From 7ab79d6de60f3aacf22c21b35a245fa9f74a6b0f Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sun, 6 Dec 2015 19:57:20 +0100 Subject: [PATCH 05/39] Add Failure documentation. --- src/cache.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 8724766..1b8141e 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -21,6 +21,10 @@ impl<'conn> StatementCache<'conn> { /// Search the cache for a prepared-statement object that implements `sql`. // If no such prepared-statement can be found, allocate and prepare a new one. + /// + /// # Failure + /// + /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. pub fn get(&mut self, sql: &str) -> SqliteResult> { let stmt = self.cache.remove(sql); match stmt { @@ -32,11 +36,16 @@ impl<'conn> StatementCache<'conn> { /// If `discard` is true, then the statement is deleted immediately. /// Otherwise it is added to the LRU list and may be returned /// by a subsequent call to `get()`. + /// + /// # Failure + /// + /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed + /// and the underlying SQLite finalize call fails. pub fn release(&mut self, stmt: SqliteStatement<'conn>, discard: bool) -> SqliteResult<()> { if discard { return stmt.finalize(); } - // FIXME stmt.reset_if_needed(); + stmt.reset_if_needed(); // clear bindings ??? self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) } From 880a78ae83293d68b7a245fcf02ca4083c7a3022 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Tue, 15 Dec 2015 21:49:59 +0100 Subject: [PATCH 06/39] Partial fix following John suggestions. --- src/cache.rs | 34 +++++++++++++++++++++++++++------- src/lib.rs | 10 +++++++++- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index d0eeede..4f7087c 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -49,22 +49,27 @@ impl<'conn> StatementCache<'conn> { return stmt.finalize(); } stmt.reset_if_needed(); - // clear bindings ??? + stmt.clear_bindings(); self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) } /// Flush the prepared statement cache - pub fn flush(&mut self) { + pub fn clear(&mut self) { self.cache.clear(); } - /// Return (current, max) sizes. - pub fn size(&self) -> (usize, usize) { - (self.cache.len(), self.cache.capacity()) + /// Return current cache size. + pub fn len(&self) -> usize { + self.cache.len() + } + + /// Return maximum cache size. + pub fn capacity(&self) -> usize { + self.cache.capacity() } /// Set the maximum number of cached statements. - pub fn set_size(&mut self, capacity: usize) { + pub fn set_capacity(&mut self, capacity: usize) { self.cache.set_capacity(capacity); } } @@ -78,13 +83,28 @@ mod test { fn test_cache() { let db = Connection::open_in_memory().unwrap(); let mut cache = StatementCache::new(&db, 10); + assert_eq!(0, cache.len()); + assert_eq!(10, cache.capacity()); + let sql = "PRAGMA schema_version"; let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + // println!("NEW {:?}", stmt); cache.release(stmt, false).unwrap(); + assert_eq!(1, cache.len()); + stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + // println!("CACHED {:?}", stmt); cache.release(stmt, true).unwrap(); - cache.flush(); + assert_eq!(0, cache.len()); + + cache.clear(); + assert_eq!(0, cache.len()); + assert_eq!(10, cache.capacity()); } } diff --git a/src/lib.rs b/src/lib.rs index 81ff23c..2ea017d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -929,7 +929,15 @@ impl<'conn> Statement<'conn> { } } - fn sql(&self) -> String { // TODO Maybe SQL should by kept as an SqliteStatement field ? + #[cfg(feature = "cache")] + fn clear_bindings(&mut self) { + unsafe { + ffi::sqlite3_clear_bindings(self.stmt); + }; + } + + #[cfg(feature = "cache")] + fn sql(&self) -> String { unsafe { let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); let utf8_str = str::from_utf8(c_slice); From 9257987b375bb07a9673bf8fba664d54ae0f3ab8 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 16 Dec 2015 19:42:03 +0100 Subject: [PATCH 07/39] Try to introduce a CachedStatement struct. --- src/cache.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 4f7087c..38a5e38 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -9,10 +9,28 @@ use self::lru_cache::LruCache; /// FIXME limitation: the same SQL can be cached only once... #[derive(Debug)] pub struct StatementCache<'conn> { - pub conn: &'conn Connection, + conn: &'conn Connection, cache: LruCache>, } +pub struct CachedStatement<'conn> { + stmt: Statement<'conn>, + cache: &'conn StatementCache<'conn>, + pub cacheable : bool, +} + +impl<'conn> Drop for CachedStatement<'conn> { + #[allow(unused_must_use)] + fn drop(&mut self) { + if self.cacheable { + // FIXME cannot borrow immutable borrowed content `*self.cache` as mutable + //self.cache.release(self.stmt, false); + } else { + self.stmt.finalize_(); + } + } +} + impl<'conn> StatementCache<'conn> { /// Create a statement cache. pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { From ff02213b531d5346b1ee9c1a252f266a1849db4f Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 16 Dec 2015 20:10:31 +0100 Subject: [PATCH 08/39] Introduce a RefCell in CachedStatement. --- src/cache.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 38a5e38..aa78e5a 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,6 +1,7 @@ //! Prepared statements cache for faster execution. extern crate lru_cache; +use std::cell::RefCell; use {Result, Connection, Statement}; use self::lru_cache::LruCache; @@ -15,7 +16,7 @@ pub struct StatementCache<'conn> { pub struct CachedStatement<'conn> { stmt: Statement<'conn>, - cache: &'conn StatementCache<'conn>, + cache: RefCell>, pub cacheable : bool, } @@ -23,8 +24,8 @@ impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { if self.cacheable { - // FIXME cannot borrow immutable borrowed content `*self.cache` as mutable - //self.cache.release(self.stmt, false); + // FIXME: cannot move out of type `cache::CachedStatement<'conn>`, which defines the `Drop` trait [E0509] + //self.cache.borrow_mut().release(self.stmt, false); } else { self.stmt.finalize_(); } From 109c26fea4100122d6819842a6069ca6993ebe99 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Thu, 17 Dec 2015 20:02:49 +0100 Subject: [PATCH 09/39] Replace LruCache by VecDeque. --- Cargo.toml | 6 +----- src/cache.rs | 54 ++++++++++++++++++++++------------------------------ src/lib.rs | 5 ++--- 3 files changed, 26 insertions(+), 39 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d077e66..ea70aea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ name = "rusqlite" load_extension = ["libsqlite3-sys/load_extension"] backup = [] blob = [] -cache = ["lru-cache"] +cache = [] functions = [] trace = [] @@ -34,10 +34,6 @@ regex = "~0.1.41" path = "libsqlite3-sys" version = "0.3.0" -[dependencies.lru-cache] -version = "~0.0.4" -optional = true - [[test]] name = "config_log" harness = false diff --git a/src/cache.rs b/src/cache.rs index aa78e5a..e51ef2c 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,33 +1,29 @@ //! Prepared statements cache for faster execution. -extern crate lru_cache; use std::cell::RefCell; +use std::collections::VecDeque; use {Result, Connection, Statement}; -use self::lru_cache::LruCache; /// Prepared statements cache. -/// -/// FIXME limitation: the same SQL can be cached only once... #[derive(Debug)] pub struct StatementCache<'conn> { conn: &'conn Connection, - cache: LruCache>, + cache: VecDeque>, // back = LRU } pub struct CachedStatement<'conn> { - stmt: Statement<'conn>, + stmt: Option>, cache: RefCell>, - pub cacheable : bool, + pub cacheable: bool, } impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { if self.cacheable { - // FIXME: cannot move out of type `cache::CachedStatement<'conn>`, which defines the `Drop` trait [E0509] - //self.cache.borrow_mut().release(self.stmt, false); + self.cache.borrow_mut().release(self.stmt.take().unwrap()); } else { - self.stmt.finalize_(); + self.stmt.take().unwrap().finalize(); } } } @@ -37,7 +33,7 @@ impl<'conn> StatementCache<'conn> { pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { StatementCache { conn: conn, - cache: LruCache::new(capacity), + cache: VecDeque::with_capacity(capacity), } } @@ -48,9 +44,8 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. pub fn get(&mut self, sql: &str) -> Result> { - let stmt = self.cache.remove(sql); - match stmt { - Some(stmt) => Ok(stmt), + match self.cache.iter().rposition(|entry| entry.eq(sql)) { + Some(index) => Ok(self.cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), } } @@ -63,13 +58,13 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. - pub fn release(&mut self, mut stmt: Statement<'conn>, discard: bool) -> Result<()> { - if discard { - return stmt.finalize(); + fn release(&mut self, mut stmt: Statement<'conn>) { + if self.cache.capacity() == self.cache.len() { // is full + self.cache.pop_back(); // LRU dropped } stmt.reset_if_needed(); stmt.clear_bindings(); - self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) + self.cache.push_front(stmt) } /// Flush the prepared statement cache @@ -86,11 +81,6 @@ impl<'conn> StatementCache<'conn> { pub fn capacity(&self) -> usize { self.cache.capacity() } - - /// Set the maximum number of cached statements. - pub fn set_capacity(&mut self, capacity: usize) { - self.cache.set_capacity(capacity); - } } #[cfg(test)] @@ -101,29 +91,31 @@ mod test { #[test] fn test_cache() { let db = Connection::open_in_memory().unwrap(); - let mut cache = StatementCache::new(&db, 10); + let mut cache = StatementCache::new(&db, 15); assert_eq!(0, cache.len()); - assert_eq!(10, cache.capacity()); + assert_eq!(15, cache.capacity()); let sql = "PRAGMA schema_version"; let mut stmt = cache.get(sql).unwrap(); assert_eq!(0, cache.len()); - assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); // println!("NEW {:?}", stmt); - cache.release(stmt, false).unwrap(); + cache.release(stmt); assert_eq!(1, cache.len()); stmt = cache.get(sql).unwrap(); assert_eq!(0, cache.len()); - assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); // println!("CACHED {:?}", stmt); - cache.release(stmt, true).unwrap(); - assert_eq!(0, cache.len()); + cache.release(stmt); + assert_eq!(1, cache.len()); cache.clear(); assert_eq!(0, cache.len()); - assert_eq!(10, cache.capacity()); + assert_eq!(15, cache.capacity()); } } diff --git a/src/lib.rs b/src/lib.rs index 2ea017d..2382815 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -937,11 +937,10 @@ impl<'conn> Statement<'conn> { } #[cfg(feature = "cache")] - fn sql(&self) -> String { + fn eq(&self, sql: &str) -> bool { unsafe { let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); - let utf8_str = str::from_utf8(c_slice); - utf8_str.unwrap().to_string() + str::from_utf8(c_slice).unwrap().eq(sql) } } From 85fb89b28079ba5bc52462f8ecdb10b47ecbd50f Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Thu, 17 Dec 2015 20:33:34 +0100 Subject: [PATCH 10/39] Fail to create a new CachedStatement. --- src/cache.rs | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index e51ef2c..bee947a 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -2,6 +2,7 @@ use std::cell::RefCell; use std::collections::VecDeque; +use std::ops::{Deref, DerefMut}; use {Result, Connection, Statement}; /// Prepared statements cache. @@ -17,6 +18,20 @@ pub struct CachedStatement<'conn> { pub cacheable: bool, } +impl<'conn> Deref for CachedStatement<'conn> { + type Target = Statement<'conn>; + + fn deref(&self) -> &Statement<'conn> { + self.stmt.as_ref().unwrap() + } +} + +impl<'conn> DerefMut for CachedStatement<'conn> { + fn deref_mut(&mut self) -> &mut Statement<'conn> { + self.stmt.as_mut().unwrap() + } +} + impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { @@ -28,6 +43,18 @@ impl<'conn> Drop for CachedStatement<'conn> { } } +impl<'conn> CachedStatement<'conn> { + fn new(stmt: Statement<'conn>, + cache: RefCell>) + -> CachedStatement<'conn> { + CachedStatement { + stmt: Some(stmt), + cache: cache, + cacheable: true, + } + } +} + impl<'conn> StatementCache<'conn> { /// Create a statement cache. pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { @@ -43,11 +70,12 @@ impl<'conn> StatementCache<'conn> { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get(&mut self, sql: &str) -> Result> { - match self.cache.iter().rposition(|entry| entry.eq(sql)) { + pub fn get(&mut self, sql: &str) -> Result> { + let stmt = match self.cache.iter().rposition(|entry| entry.eq(sql)) { Some(index) => Ok(self.cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), - } + }; + stmt.map(|stmt| CachedStatement::new(stmt, RefCell::new(self))) } /// If `discard` is true, then the statement is deleted immediately. @@ -59,7 +87,8 @@ impl<'conn> StatementCache<'conn> { /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. fn release(&mut self, mut stmt: Statement<'conn>) { - if self.cache.capacity() == self.cache.len() { // is full + if self.cache.capacity() == self.cache.len() { + // is full self.cache.pop_back(); // LRU dropped } stmt.reset_if_needed(); From 30c8910d1920e822e0790325852faee4d0faec53 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Fri, 18 Dec 2015 20:18:46 +0100 Subject: [PATCH 11/39] Still some lifetime problem... --- src/cache.rs | 62 ++++++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index bee947a..f2e050e 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -9,12 +9,12 @@ use {Result, Connection, Statement}; #[derive(Debug)] pub struct StatementCache<'conn> { conn: &'conn Connection, - cache: VecDeque>, // back = LRU + cache: RefCell>>, // back = LRU } pub struct CachedStatement<'conn> { stmt: Option>, - cache: RefCell>, + cache: &'conn StatementCache<'conn>, pub cacheable: bool, } @@ -36,7 +36,7 @@ impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { if self.cacheable { - self.cache.borrow_mut().release(self.stmt.take().unwrap()); + self.cache.release(self.stmt.take().unwrap()); } else { self.stmt.take().unwrap().finalize(); } @@ -44,9 +44,7 @@ impl<'conn> Drop for CachedStatement<'conn> { } impl<'conn> CachedStatement<'conn> { - fn new(stmt: Statement<'conn>, - cache: RefCell>) - -> CachedStatement<'conn> { + fn new(stmt: Statement<'conn>, cache: &'conn StatementCache<'conn>) -> CachedStatement<'conn> { CachedStatement { stmt: Some(stmt), cache: cache, @@ -60,7 +58,7 @@ impl<'conn> StatementCache<'conn> { pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { StatementCache { conn: conn, - cache: VecDeque::with_capacity(capacity), + cache: RefCell::new(VecDeque::with_capacity(capacity)), } } @@ -70,12 +68,12 @@ impl<'conn> StatementCache<'conn> { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get(&mut self, sql: &str) -> Result> { - let stmt = match self.cache.iter().rposition(|entry| entry.eq(sql)) { - Some(index) => Ok(self.cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant + pub fn get(&'conn self, sql: &str) -> Result> { + let stmt = match self.cache.borrow().iter().rposition(|entry| entry.eq(sql)) { + Some(index) => Ok(self.cache.borrow_mut().swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), }; - stmt.map(|stmt| CachedStatement::new(stmt, RefCell::new(self))) + stmt.map(|stmt| CachedStatement::new(stmt, self)) } /// If `discard` is true, then the statement is deleted immediately. @@ -86,29 +84,29 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. - fn release(&mut self, mut stmt: Statement<'conn>) { - if self.cache.capacity() == self.cache.len() { + fn release(&self, mut stmt: Statement<'conn>) { + if self.cache.borrow().capacity() == self.cache.borrow().len() { // is full - self.cache.pop_back(); // LRU dropped + self.cache.borrow_mut().pop_back(); // LRU dropped } stmt.reset_if_needed(); stmt.clear_bindings(); - self.cache.push_front(stmt) + self.cache.borrow_mut().push_front(stmt) } /// Flush the prepared statement cache - pub fn clear(&mut self) { - self.cache.clear(); + pub fn clear(&self) { + self.cache.borrow_mut().clear(); } /// Return current cache size. pub fn len(&self) -> usize { - self.cache.len() + self.cache.borrow().len() } /// Return maximum cache size. pub fn capacity(&self) -> usize { - self.cache.capacity() + self.cache.borrow().capacity() } } @@ -125,22 +123,20 @@ mod test { assert_eq!(15, cache.capacity()); let sql = "PRAGMA schema_version"; - let mut stmt = cache.get(sql).unwrap(); - assert_eq!(0, cache.len()); - assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); - - // println!("NEW {:?}", stmt); - cache.release(stmt); + { + let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + } assert_eq!(1, cache.len()); - stmt = cache.get(sql).unwrap(); - assert_eq!(0, cache.len()); - assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); - - // println!("CACHED {:?}", stmt); - cache.release(stmt); + { + let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + } assert_eq!(1, cache.len()); cache.clear(); From 8d84e2b0764215734ca39c5b9146bf85c10212e8 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Fri, 18 Dec 2015 20:21:41 +0100 Subject: [PATCH 12/39] Activate cache feature in travis. --- .travis.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2bd2336..197fc9b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,8 +12,9 @@ script: - cargo test --features load_extension - cargo test --features trace - cargo test --features functions - - cargo test --features "backup functions load_extension trace" - - cargo doc --no-deps --features "backup functions load_extension trace" + - cargo test --features cache + - cargo test --features "backup functions load_extension trace cache" + - cargo doc --no-deps --features "backup functions load_extension trace cache" after_success: | [ $TRAVIS_BRANCH = master ] && From 7bdf80ccdb6c22907753441d6ef9420987c04e83 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 16:49:11 +0100 Subject: [PATCH 13/39] Introduce two distinct lifetime parameters. --- src/cache.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index f2e050e..e2b1c67 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -12,27 +12,27 @@ pub struct StatementCache<'conn> { cache: RefCell>>, // back = LRU } -pub struct CachedStatement<'conn> { - stmt: Option>, - cache: &'conn StatementCache<'conn>, +pub struct CachedStatement<'c: 's, 's> { + stmt: Option>, + cache: &'s StatementCache<'c>, pub cacheable: bool, } -impl<'conn> Deref for CachedStatement<'conn> { - type Target = Statement<'conn>; +impl<'c, 's> Deref for CachedStatement<'c, 's> { + type Target = Statement<'c>; - fn deref(&self) -> &Statement<'conn> { + fn deref(&self) -> &Statement<'c> { self.stmt.as_ref().unwrap() } } -impl<'conn> DerefMut for CachedStatement<'conn> { - fn deref_mut(&mut self) -> &mut Statement<'conn> { +impl<'c, 's> DerefMut for CachedStatement<'c, 's> { + fn deref_mut(&mut self) -> &mut Statement<'c> { self.stmt.as_mut().unwrap() } } -impl<'conn> Drop for CachedStatement<'conn> { +impl<'c, 's> Drop for CachedStatement<'c, 's> { #[allow(unused_must_use)] fn drop(&mut self) { if self.cacheable { @@ -43,8 +43,8 @@ impl<'conn> Drop for CachedStatement<'conn> { } } -impl<'conn> CachedStatement<'conn> { - fn new(stmt: Statement<'conn>, cache: &'conn StatementCache<'conn>) -> CachedStatement<'conn> { +impl<'c, 's> CachedStatement<'c, 's> { + fn new(stmt: Statement<'c>, cache: &'s StatementCache<'c>) -> CachedStatement<'c, 's> { CachedStatement { stmt: Some(stmt), cache: cache, @@ -68,7 +68,7 @@ impl<'conn> StatementCache<'conn> { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get(&'conn self, sql: &str) -> Result> { + pub fn get<'s>(&'s self, sql: &str) -> Result> { let stmt = match self.cache.borrow().iter().rposition(|entry| entry.eq(sql)) { Some(index) => Ok(self.cache.borrow_mut().swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), From cb1951c21a25adc660e3c9a02ee2e7a17c9e8fb5 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 16:56:41 +0100 Subject: [PATCH 14/39] Fix borrowing --- src/cache.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index e2b1c67..f7599ca 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -69,8 +69,9 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. pub fn get<'s>(&'s self, sql: &str) -> Result> { - let stmt = match self.cache.borrow().iter().rposition(|entry| entry.eq(sql)) { - Some(index) => Ok(self.cache.borrow_mut().swap_remove_front(index).unwrap()), // FIXME Not LRU compliant + let mut cache = self.cache.borrow_mut(); + let stmt = match cache.iter().rposition(|entry| entry.eq(sql)) { + Some(index) => Ok(cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), }; stmt.map(|stmt| CachedStatement::new(stmt, self)) @@ -85,13 +86,14 @@ impl<'conn> StatementCache<'conn> { /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. fn release(&self, mut stmt: Statement<'conn>) { - if self.cache.borrow().capacity() == self.cache.borrow().len() { + let mut cache = self.cache.borrow_mut(); + if cache.capacity() == cache.len() { // is full - self.cache.borrow_mut().pop_back(); // LRU dropped + cache.pop_back(); // LRU dropped } stmt.reset_if_needed(); stmt.clear_bindings(); - self.cache.borrow_mut().push_front(stmt) + cache.push_front(stmt) } /// Flush the prepared statement cache @@ -118,7 +120,7 @@ mod test { #[test] fn test_cache() { let db = Connection::open_in_memory().unwrap(); - let mut cache = StatementCache::new(&db, 15); + let cache = StatementCache::new(&db, 15); assert_eq!(0, cache.len()); assert_eq!(15, cache.capacity()); From 5876be3d480694f0faf7e1e4d94c3c79a9e9384c Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 17:01:06 +0100 Subject: [PATCH 15/39] Add test with cacheable set to false --- src/cache.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/cache.rs b/src/cache.rs index f7599ca..1173901 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -145,4 +145,20 @@ mod test { assert_eq!(0, cache.len()); assert_eq!(15, cache.capacity()); } + + #[test] + fn test_cacheable() { + let db = Connection::open_in_memory().unwrap(); + let cache = StatementCache::new(&db, 15); + + let sql = "PRAGMA schema_version"; + { + let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.cacheable = false; + } + assert_eq!(0, cache.len()); + } } From 68b4943a39d36d207ad86ada8fd4f6c4c05ad04e Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 17:14:06 +0100 Subject: [PATCH 16/39] Add some doc --- src/cache.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 1173901..a0cce77 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -5,13 +5,17 @@ use std::collections::VecDeque; use std::ops::{Deref, DerefMut}; use {Result, Connection, Statement}; -/// Prepared statements cache. +/// Prepared statements LRU cache. #[derive(Debug)] pub struct StatementCache<'conn> { conn: &'conn Connection, cache: RefCell>>, // back = LRU } +/// Cacheable statement. +/// +/// Statement will return automatically to the cache by default. +/// If you want the statement to be discarded, you can set the `cacheable` flag to `false`. pub struct CachedStatement<'c: 's, 's> { stmt: Option>, cache: &'s StatementCache<'c>, From 1ec2dee5332f6acd40820141daa709f77ff0fe6e Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 17:22:54 +0100 Subject: [PATCH 17/39] Ensure features documentation is generated. --- .travis.yml | 2 +- publish-ghp-docs.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e769717..50e7585 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,4 +13,4 @@ script: - cargo test --features trace - cargo test --features functions - cargo test --features cache - - cargo test --features "backup functions load_extension trace cache" + - cargo test --features "backup cache functions load_extension trace" diff --git a/publish-ghp-docs.sh b/publish-ghp-docs.sh index f9eeb17..de31a13 100755 --- a/publish-ghp-docs.sh +++ b/publish-ghp-docs.sh @@ -8,7 +8,7 @@ fi cd $(git rev-parse --show-toplevel) rm -rf target/doc/ -multirust run nightly cargo doc --no-deps --features "load_extension trace" +multirust run nightly cargo doc --no-deps --features "backup cache functions load_extension trace" echo '' > target/doc/index.html ghp-import target/doc git push origin gh-pages:gh-pages From 9b4fdc29ee7550ebe2a498f36f6a197853274261 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 20 Dec 2015 09:46:05 +0100 Subject: [PATCH 18/39] Add benchmarks for statement cache. --- benches/lib.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 benches/lib.rs diff --git a/benches/lib.rs b/benches/lib.rs new file mode 100644 index 0000000..92fddef --- /dev/null +++ b/benches/lib.rs @@ -0,0 +1,23 @@ +#![feature(test)] +extern crate test; + +extern crate rusqlite; + +use rusqlite::Connection; +use rusqlite::cache::StatementCache; +use test::Bencher; + +#[bench] +fn bench_no_cache(b: &mut Bencher) { + let db = Connection::open_in_memory().unwrap(); + let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71"; + b.iter(|| db.prepare(sql).unwrap()); +} + +#[bench] +fn bench_cache(b: &mut Bencher) { + let db = Connection::open_in_memory().unwrap(); + let cache = StatementCache::new(&db, 15); + let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71"; + b.iter(|| cache.get(sql).unwrap()); +} From ed0923bba97f29d482edba8c44264132d4adde1d Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 7 Jan 2016 11:20:42 -0500 Subject: [PATCH 19/39] Update for RowIndex change --- src/cache.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index a0cce77..f30bc42 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -133,7 +133,7 @@ mod test { let mut stmt = cache.get(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); } assert_eq!(1, cache.len()); @@ -141,7 +141,7 @@ mod test { let mut stmt = cache.get(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); } assert_eq!(1, cache.len()); @@ -160,7 +160,7 @@ mod test { let mut stmt = cache.get(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); stmt.cacheable = false; } assert_eq!(0, cache.len()); From 7b29277d6fa6dc4a805a951020a9d03e51b424e0 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 7 Jan 2016 11:24:20 -0500 Subject: [PATCH 20/39] Use `discard()` instead of `cacheable = false` to avoid prevent cached statements from returning to the cache. --- src/cache.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index f30bc42..509f349 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -15,11 +15,10 @@ pub struct StatementCache<'conn> { /// Cacheable statement. /// /// Statement will return automatically to the cache by default. -/// If you want the statement to be discarded, you can set the `cacheable` flag to `false`. +/// If you want the statement to be discarded, call `discard()` on it. pub struct CachedStatement<'c: 's, 's> { stmt: Option>, cache: &'s StatementCache<'c>, - pub cacheable: bool, } impl<'c, 's> Deref for CachedStatement<'c, 's> { @@ -39,10 +38,8 @@ impl<'c, 's> DerefMut for CachedStatement<'c, 's> { impl<'c, 's> Drop for CachedStatement<'c, 's> { #[allow(unused_must_use)] fn drop(&mut self) { - if self.cacheable { - self.cache.release(self.stmt.take().unwrap()); - } else { - self.stmt.take().unwrap().finalize(); + if let Some(stmt) = self.stmt.take() { + self.cache.release(stmt); } } } @@ -52,9 +49,12 @@ impl<'c, 's> CachedStatement<'c, 's> { CachedStatement { stmt: Some(stmt), cache: cache, - cacheable: true, } } + + pub fn discard(mut self) { + self.stmt = None; + } } impl<'conn> StatementCache<'conn> { @@ -67,7 +67,7 @@ impl<'conn> StatementCache<'conn> { } /// Search the cache for a prepared-statement object that implements `sql`. - // If no such prepared-statement can be found, allocate and prepare a new one. + /// If no such prepared-statement can be found, allocate and prepare a new one. /// /// # Failure /// @@ -151,7 +151,7 @@ mod test { } #[test] - fn test_cacheable() { + fn test_discard() { let db = Connection::open_in_memory().unwrap(); let cache = StatementCache::new(&db, 15); @@ -161,7 +161,7 @@ mod test { assert_eq!(0, cache.len()); assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); - stmt.cacheable = false; + stmt.discard(); } assert_eq!(0, cache.len()); } From 0a371b714525488242aa9ecc9aca497a58a847c9 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 7 Jan 2016 11:30:51 -0500 Subject: [PATCH 21/39] Rename StatementCache::release -> cache_stmt --- src/cache.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 509f349..dd73d81 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -39,7 +39,7 @@ impl<'c, 's> Drop for CachedStatement<'c, 's> { #[allow(unused_must_use)] fn drop(&mut self) { if let Some(stmt) = self.stmt.take() { - self.cache.release(stmt); + self.cache.cache_stmt(stmt); } } } @@ -81,15 +81,8 @@ impl<'conn> StatementCache<'conn> { stmt.map(|stmt| CachedStatement::new(stmt, self)) } - /// If `discard` is true, then the statement is deleted immediately. - /// Otherwise it is added to the LRU list and may be returned - /// by a subsequent call to `get()`. - /// - /// # Failure - /// - /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed - /// and the underlying SQLite finalize call fails. - fn release(&self, mut stmt: Statement<'conn>) { + // Return a statement to the cache. + fn cache_stmt(&self, mut stmt: Statement<'conn>) { let mut cache = self.cache.borrow_mut(); if cache.capacity() == cache.len() { // is full From 25b554bd993f4e510dee447d9bddad05d9627738 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 7 Jan 2016 11:31:57 -0500 Subject: [PATCH 22/39] Update Changelog with cache feature --- Changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.md b/Changelog.md index 9e27d21..5c8cb5a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,7 @@ # Version UPCOMING (...) +* Adds a `cache` Cargo feature that provides `cache::StatementCache` for caching prepraed + statements. * Adds `column_count()` method to `Statement` and `Row`. * Adds `types::Value` for dynamic column types. * Introduces a `RowIndex` trait allowing columns to be fetched via index (as before) or name (new). From 840a86a883456aadfd75d043eebede85c3eae539 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 7 Jan 2016 11:32:56 -0500 Subject: [PATCH 23/39] Add blob feature to doc-publishing script --- publish-ghp-docs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/publish-ghp-docs.sh b/publish-ghp-docs.sh index de31a13..c8fc790 100755 --- a/publish-ghp-docs.sh +++ b/publish-ghp-docs.sh @@ -8,7 +8,7 @@ fi cd $(git rev-parse --show-toplevel) rm -rf target/doc/ -multirust run nightly cargo doc --no-deps --features "backup cache functions load_extension trace" +multirust run nightly cargo doc --no-deps --features "backup cache functions load_extension trace blob" echo '' > target/doc/index.html ghp-import target/doc git push origin gh-pages:gh-pages From 501d8782a528be2eff5bffa252f74d2da13c530c Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 7 Jan 2016 15:33:04 -0500 Subject: [PATCH 24/39] Fix typo in Changelog --- Changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 5c8cb5a..d4f9317 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,6 @@ # Version UPCOMING (...) -* Adds a `cache` Cargo feature that provides `cache::StatementCache` for caching prepraed +* Adds a `cache` Cargo feature that provides `cache::StatementCache` for caching prepared statements. * Adds `column_count()` method to `Statement` and `Row`. * Adds `types::Value` for dynamic column types. From ffabee7169520df9af9f5585a47ddb35492ca73c Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 2 Feb 2016 10:40:15 -0500 Subject: [PATCH 25/39] Add cache to appveyor test list --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 9fb260e..bb5b82f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -16,7 +16,7 @@ build: false test_script: - cargo test --lib --verbose - - cargo test --lib --features "backup blob functions load_extension trace" + - cargo test --lib --features "backup blob cache functions load_extension trace" cache: - C:\Users\appveyor\.cargo From f6aba80f4b07f31c33bf6f02f2c2dbf8bb1026ec Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 10:06:43 -0500 Subject: [PATCH 26/39] Extract RawStatement wrapper around *mut sqlite3_stmt. --- src/lib.rs | 74 +++++++++++++++++++------------------------- src/named_params.rs | 12 ++----- src/raw_statement.rs | 71 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 51 deletions(-) create mode 100644 src/raw_statement.rs diff --git a/src/lib.rs b/src/lib.rs index 1eb00bb..62b9106 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -76,6 +76,7 @@ use libc::{c_int, c_char}; use types::{ToSql, FromSql}; use error::{error_from_sqlite_code, error_from_handle}; +use raw_statement::RawStatement; pub use transaction::{SqliteTransaction, Transaction, TransactionBehavior}; pub use error::{SqliteError, Error}; @@ -88,6 +89,7 @@ mod transaction; mod named_params; mod error; mod convenient; +mod raw_statement; #[cfg(feature = "load_extension")]mod load_extension_guard; #[cfg(feature = "trace")]pub mod trace; #[cfg(feature = "backup")]pub mod backup; @@ -687,7 +689,7 @@ impl InnerConnection { &mut c_stmt, ptr::null_mut()) }; - self.decode_result(r).map(|_| Statement::new(conn, c_stmt)) + self.decode_result(r).map(|_| Statement::new(conn, RawStatement::new(c_stmt))) } fn changes(&mut self) -> c_int { @@ -708,16 +710,17 @@ pub type SqliteStatement<'conn> = Statement<'conn>; /// A prepared statement. pub struct Statement<'conn> { conn: &'conn Connection, - stmt: *mut ffi::sqlite3_stmt, + stmt: RawStatement, column_count: c_int, } impl<'conn> Statement<'conn> { - fn new(conn: &Connection, stmt: *mut ffi::sqlite3_stmt) -> Statement { + fn new(conn: &Connection, stmt: RawStatement) -> Statement { + let column_count = stmt.column_count(); Statement { conn: conn, stmt: stmt, - column_count: unsafe { ffi::sqlite3_column_count(stmt) }, + column_count: column_count, } } @@ -726,7 +729,7 @@ impl<'conn> Statement<'conn> { let n = self.column_count; let mut cols = Vec::with_capacity(n as usize); for i in 0..n { - let slice = unsafe { CStr::from_ptr(ffi::sqlite3_column_name(self.stmt, i)) }; + let slice = self.stmt.column_name(i); let s = str::from_utf8(slice.to_bytes()).unwrap(); cols.push(s); } @@ -747,8 +750,7 @@ impl<'conn> Statement<'conn> { let bytes = name.as_bytes(); let n = self.column_count; for i in 0..n { - let slice = unsafe { CStr::from_ptr(ffi::sqlite3_column_name(self.stmt, i)) }; - if bytes == slice.to_bytes() { + if bytes == self.stmt.column_name(i).to_bytes() { return Ok(i); } } @@ -779,15 +781,13 @@ impl<'conn> Statement<'conn> { /// Will return `Err` if binding parameters fails, the executed statement returns rows (in /// which case `query` should be used instead), or the underling SQLite call fails. pub fn execute(&mut self, params: &[&ToSql]) -> Result { - unsafe { - try!(self.bind_parameters(params)); - self.execute_() - } + try!(self.bind_parameters(params)); + self.execute_() } - unsafe fn execute_(&mut self) -> Result { - let r = ffi::sqlite3_step(self.stmt); - ffi::sqlite3_reset(self.stmt); + fn execute_(&mut self) -> Result { + let r = self.stmt.step(); + self.stmt.reset(); match r { ffi::SQLITE_DONE => { if self.column_count == 0 { @@ -825,10 +825,7 @@ impl<'conn> Statement<'conn> { /// /// Will return `Err` if binding parameters fails. pub fn query<'a>(&'a mut self, params: &[&ToSql]) -> Result> { - unsafe { - try!(self.bind_parameters(params)); - } - + try!(self.bind_parameters(params)); Ok(Rows::new(self)) } @@ -889,14 +886,16 @@ impl<'conn> Statement<'conn> { self.finalize_() } - unsafe fn bind_parameters(&mut self, params: &[&ToSql]) -> Result<()> { - assert!(params.len() as c_int == ffi::sqlite3_bind_parameter_count(self.stmt), + fn bind_parameters(&mut self, params: &[&ToSql]) -> Result<()> { + assert!(params.len() as c_int == self.stmt.bind_parameter_count(), "incorrect number of parameters to query(): expected {}, got {}", - ffi::sqlite3_bind_parameter_count(self.stmt), + self.stmt.bind_parameter_count(), params.len()); for (i, p) in params.iter().enumerate() { - try!(self.conn.decode_result(p.bind_parameter(self.stmt, (i + 1) as c_int))); + try!(unsafe { + self.conn.decode_result(p.bind_parameter(self.stmt.ptr(), (i + 1) as c_int)) + }); } Ok(()) @@ -904,32 +903,25 @@ impl<'conn> Statement<'conn> { #[cfg(feature = "cache")] fn clear_bindings(&mut self) { - unsafe { - ffi::sqlite3_clear_bindings(self.stmt); - }; + self.stmt.clear_bindings(); } #[cfg(feature = "cache")] fn eq(&self, sql: &str) -> bool { - unsafe { - let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); - str::from_utf8(c_slice).unwrap().eq(sql) - } + let c_slice = self.stmt.sql().to_bytes(); + sql.as_bytes().eq(c_slice) } fn finalize_(&mut self) -> Result<()> { - let r = unsafe { ffi::sqlite3_finalize(self.stmt) }; - self.stmt = ptr::null_mut(); - self.conn.decode_result(r) + let mut stmt = RawStatement::new(ptr::null_mut()); + mem::swap(&mut stmt, &mut self.stmt); + self.conn.decode_result(stmt.finalize()) } } impl<'conn> fmt::Debug for Statement<'conn> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let sql = unsafe { - let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); - str::from_utf8(c_slice) - }; + let sql = str::from_utf8(self.stmt.sql().to_bytes()); f.debug_struct("Statement") .field("conn", self.conn) .field("stmt", &self.stmt) @@ -1039,9 +1031,7 @@ impl<'stmt> Rows<'stmt> { fn reset(&mut self) { if let Some(stmt) = self.stmt.take() { - unsafe { - ffi::sqlite3_reset(stmt.stmt); - } + stmt.stmt.reset(); } } } @@ -1051,7 +1041,7 @@ impl<'stmt> Iterator for Rows<'stmt> { fn next(&mut self) -> Option>> { self.stmt.and_then(|stmt| { - match unsafe { ffi::sqlite3_step(stmt.stmt) } { + match stmt.stmt.step() { ffi::SQLITE_ROW => { let current_row = self.current_row.get() + 1; self.current_row.set(current_row); @@ -1146,8 +1136,8 @@ impl<'stmt> Row<'stmt> { unsafe { let idx = try!(idx.idx(self.stmt)); - if T::column_has_valid_sqlite_type(self.stmt.stmt, idx) { - FromSql::column_result(self.stmt.stmt, idx) + if T::column_has_valid_sqlite_type(self.stmt.stmt.ptr(), idx) { + FromSql::column_result(self.stmt.stmt.ptr(), idx) } else { Err(Error::InvalidColumnType) } diff --git a/src/named_params.rs b/src/named_params.rs index 3e62610..a881646 100644 --- a/src/named_params.rs +++ b/src/named_params.rs @@ -1,7 +1,5 @@ use libc::c_int; -use super::ffi; - use {Result, Error, Connection, Statement, Rows, Row, str_to_cstring}; use types::ToSql; @@ -56,11 +54,7 @@ impl<'conn> Statement<'conn> { /// is valid but not a bound parameter of this statement. pub fn parameter_index(&self, name: &str) -> Result> { let c_name = try!(str_to_cstring(name)); - let c_index = unsafe { ffi::sqlite3_bind_parameter_index(self.stmt, c_name.as_ptr()) }; - Ok(match c_index { - 0 => None, // A zero is returned if no matching parameter is found. - n => Some(n), - }) + Ok(self.stmt.bind_parameter_index(&c_name)) } /// Execute the prepared statement with named parameter(s). If any parameters @@ -87,7 +81,7 @@ impl<'conn> Statement<'conn> { /// which case `query` should be used instead), or the underling SQLite call fails. pub fn execute_named(&mut self, params: &[(&str, &ToSql)]) -> Result { try!(self.bind_parameters_named(params)); - unsafe { self.execute_() } + self.execute_() } /// Execute the prepared statement with named parameter(s), returning an iterator over the @@ -120,7 +114,7 @@ impl<'conn> Statement<'conn> { fn bind_parameters_named(&mut self, params: &[(&str, &ToSql)]) -> Result<()> { for &(name, value) in params { if let Some(i) = try!(self.parameter_index(name)) { - try!(self.conn.decode_result(unsafe { value.bind_parameter(self.stmt, i) })); + try!(self.conn.decode_result(unsafe { value.bind_parameter(self.stmt.ptr(), i) })); } else { return Err(Error::InvalidParameterName(name.into())); } diff --git a/src/raw_statement.rs b/src/raw_statement.rs new file mode 100644 index 0000000..896508e --- /dev/null +++ b/src/raw_statement.rs @@ -0,0 +1,71 @@ +use std::ffi::CStr; +use std::ptr; +use libc::c_int; +use super::ffi; + +// Private newtype for raw sqlite3_stmts that finalize themselves when dropped. +#[derive(Debug)] +pub struct RawStatement(*mut ffi::sqlite3_stmt); + +impl RawStatement { + pub fn new(stmt: *mut ffi::sqlite3_stmt) -> RawStatement { + RawStatement(stmt) + } + + pub unsafe fn ptr(&self) -> *mut ffi::sqlite3_stmt { + self.0 + } + + pub fn column_count(&self) -> c_int { + unsafe { ffi::sqlite3_column_count(self.0) } + } + + pub fn column_name(&self, idx: c_int) -> &CStr { + unsafe { CStr::from_ptr(ffi::sqlite3_column_name(self.0, idx)) } + } + + pub fn step(&self) -> c_int { + unsafe { ffi::sqlite3_step(self.0) } + } + + pub fn reset(&self) -> c_int { + unsafe { ffi::sqlite3_reset(self.0) } + } + + pub fn bind_parameter_count(&self) -> c_int { + unsafe { ffi::sqlite3_bind_parameter_count(self.0) } + } + + pub fn bind_parameter_index(&self, name: &CStr) -> Option { + let r = unsafe { ffi::sqlite3_bind_parameter_index(self.0, name.as_ptr()) }; + match r { + 0 => None, + i => Some(i), + } + } + + #[cfg(feature = "cache")] + pub fn clear_bindings(&self) -> c_int { + unsafe { ffi::sqlite3_clear_bindings(self.0) } + } + + pub fn sql(&self) -> &CStr { + unsafe { CStr::from_ptr(ffi::sqlite3_sql(self.0)) } + } + + pub fn finalize(mut self) -> c_int { + self.finalize_() + } + + fn finalize_(&mut self) -> c_int { + let r = unsafe { ffi::sqlite3_finalize(self.0) }; + self.0 = ptr::null_mut(); + r + } +} + +impl Drop for RawStatement { + fn drop(&mut self) { + self.finalize_(); + } +} From 1978568d014ece577c03cb0b563f77585d6eea7d Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 11:55:10 -0500 Subject: [PATCH 27/39] Make StatementCache hold RawStatements instead of Statements. --- src/cache.rs | 14 +++++++++----- src/lib.rs | 19 ++++++++----------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 28b047d..5522f06 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -4,12 +4,13 @@ use std::cell::RefCell; use std::collections::VecDeque; use std::ops::{Deref, DerefMut}; use {Result, Connection, Statement}; +use raw_statement::RawStatement; /// Prepared statements LRU cache. #[derive(Debug)] pub struct StatementCache<'conn> { conn: &'conn Connection, - cache: RefCell>>, // back = LRU + cache: RefCell>, // back = LRU } /// Cacheable statement. @@ -39,7 +40,7 @@ impl<'c, 's> Drop for CachedStatement<'c, 's> { #[allow(unused_must_use)] fn drop(&mut self) { if let Some(stmt) = self.stmt.take() { - self.cache.cache_stmt(stmt); + self.cache.cache_stmt(stmt.into()); } } } @@ -74,15 +75,18 @@ impl<'conn> StatementCache<'conn> { /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. pub fn get<'s>(&'s self, sql: &str) -> Result> { let mut cache = self.cache.borrow_mut(); - let stmt = match cache.iter().rposition(|entry| entry.eq(sql)) { - Some(index) => Ok(cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant + let stmt = match cache.iter().rposition(|entry| entry.sql().to_bytes().eq(sql.as_bytes())) { + Some(index) => { + let raw_stmt = cache.swap_remove_front(index).unwrap(); // FIXME Not LRU compliant + Ok(Statement::new(self.conn, raw_stmt)) + } _ => self.conn.prepare(sql), }; stmt.map(|stmt| CachedStatement::new(stmt, self)) } // Return a statement to the cache. - fn cache_stmt(&self, mut stmt: Statement<'conn>) { + fn cache_stmt(&self, stmt: RawStatement) { let mut cache = self.cache.borrow_mut(); if cache.capacity() == cache.len() { // is full diff --git a/src/lib.rs b/src/lib.rs index 62b9106..8512857 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -901,17 +901,6 @@ impl<'conn> Statement<'conn> { Ok(()) } - #[cfg(feature = "cache")] - fn clear_bindings(&mut self) { - self.stmt.clear_bindings(); - } - - #[cfg(feature = "cache")] - fn eq(&self, sql: &str) -> bool { - let c_slice = self.stmt.sql().to_bytes(); - sql.as_bytes().eq(c_slice) - } - fn finalize_(&mut self) -> Result<()> { let mut stmt = RawStatement::new(ptr::null_mut()); mem::swap(&mut stmt, &mut self.stmt); @@ -919,6 +908,14 @@ impl<'conn> Statement<'conn> { } } +impl<'conn> Into for Statement<'conn> { + fn into(mut self) -> RawStatement { + let mut stmt = RawStatement::new(ptr::null_mut()); + mem::swap(&mut stmt, &mut self.stmt); + stmt + } +} + impl<'conn> fmt::Debug for Statement<'conn> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let sql = str::from_utf8(self.stmt.sql().to_bytes()); From 0ab9421e6adedfcf7fb37473bdf2c778fd5b4302 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 11:59:54 -0500 Subject: [PATCH 28/39] Detach StatementCache from Connection so we can embed it (coming later) --- src/cache.rs | 28 +++++++++++++--------------- src/lib.rs | 2 +- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 5522f06..8d7ac06 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -8,8 +8,7 @@ use raw_statement::RawStatement; /// Prepared statements LRU cache. #[derive(Debug)] -pub struct StatementCache<'conn> { - conn: &'conn Connection, +pub struct StatementCache { cache: RefCell>, // back = LRU } @@ -19,7 +18,7 @@ pub struct StatementCache<'conn> { /// If you want the statement to be discarded, call `discard()` on it. pub struct CachedStatement<'c: 's, 's> { stmt: Option>, - cache: &'s StatementCache<'c>, + cache: &'s StatementCache, } impl<'c, 's> Deref for CachedStatement<'c, 's> { @@ -46,7 +45,7 @@ impl<'c, 's> Drop for CachedStatement<'c, 's> { } impl<'c, 's> CachedStatement<'c, 's> { - fn new(stmt: Statement<'c>, cache: &'s StatementCache<'c>) -> CachedStatement<'c, 's> { + fn new(stmt: Statement<'c>, cache: &'s StatementCache) -> CachedStatement<'c, 's> { CachedStatement { stmt: Some(stmt), cache: cache, @@ -58,11 +57,10 @@ impl<'c, 's> CachedStatement<'c, 's> { } } -impl<'conn> StatementCache<'conn> { +impl StatementCache { /// Create a statement cache. - pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { + pub fn with_capacity(capacity: usize) -> StatementCache { StatementCache { - conn: conn, cache: RefCell::new(VecDeque::with_capacity(capacity)), } } @@ -73,14 +71,14 @@ impl<'conn> StatementCache<'conn> { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get<'s>(&'s self, sql: &str) -> Result> { + pub fn get<'conn, 's>(&'s self, conn: &'conn Connection, sql: &str) -> Result> { let mut cache = self.cache.borrow_mut(); let stmt = match cache.iter().rposition(|entry| entry.sql().to_bytes().eq(sql.as_bytes())) { Some(index) => { let raw_stmt = cache.swap_remove_front(index).unwrap(); // FIXME Not LRU compliant - Ok(Statement::new(self.conn, raw_stmt)) + Ok(Statement::new(conn, raw_stmt)) } - _ => self.conn.prepare(sql), + _ => conn.prepare(sql), }; stmt.map(|stmt| CachedStatement::new(stmt, self)) } @@ -120,13 +118,13 @@ mod test { #[test] fn test_cache() { let db = Connection::open_in_memory().unwrap(); - let cache = StatementCache::new(&db, 15); + let cache = StatementCache::with_capacity(15); assert_eq!(0, cache.len()); assert_eq!(15, cache.capacity()); let sql = "PRAGMA schema_version"; { - let mut stmt = cache.get(sql).unwrap(); + let mut stmt = cache.get(&db, sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); @@ -134,7 +132,7 @@ mod test { assert_eq!(1, cache.len()); { - let mut stmt = cache.get(sql).unwrap(); + let mut stmt = cache.get(&db, sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); @@ -149,11 +147,11 @@ mod test { #[test] fn test_discard() { let db = Connection::open_in_memory().unwrap(); - let cache = StatementCache::new(&db, 15); + let cache = StatementCache::with_capacity(15); let sql = "PRAGMA schema_version"; { - let mut stmt = cache.get(sql).unwrap(); + let mut stmt = cache.get(&db, sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); diff --git a/src/lib.rs b/src/lib.rs index 8512857..15e38db 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -93,7 +93,7 @@ mod raw_statement; #[cfg(feature = "load_extension")]mod load_extension_guard; #[cfg(feature = "trace")]pub mod trace; #[cfg(feature = "backup")]pub mod backup; -#[cfg(feature = "cache")]pub mod cache; +#[cfg(feature = "cache")]mod cache; #[cfg(feature = "functions")]pub mod functions; #[cfg(feature = "blob")]pub mod blob; From ed72da92ef323721d7c3af27a28eebfff0f88bc9 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 12:01:55 -0500 Subject: [PATCH 29/39] Remove cache feature --- .travis.yml | 3 +-- Cargo.toml | 1 - appveyor.yml | 2 +- src/lib.rs | 3 ++- src/raw_statement.rs | 1 - 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index ae6427c..d520dab 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,6 @@ script: - cargo test --features load_extension - cargo test --features trace - cargo test --features functions - - cargo test --features cache - cargo test --features chrono - cargo test --features serde_json - - cargo test --features "backup blob cache chrono functions load_extension serde_json trace" + - cargo test --features "backup blob chrono functions load_extension serde_json trace" diff --git a/Cargo.toml b/Cargo.toml index 6d6f717..ccabe9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,6 @@ name = "rusqlite" load_extension = [] backup = [] blob = [] -cache = [] functions = [] trace = [] diff --git a/appveyor.yml b/appveyor.yml index 329665d..e15cb90 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -16,7 +16,7 @@ build: false test_script: - cargo test --lib --verbose - - cargo test --lib --features "backup blob cache chrono functions load_extension serde_json trace" + - cargo test --lib --features "backup blob chrono functions load_extension serde_json trace" cache: - C:\Users\appveyor\.cargo diff --git a/src/lib.rs b/src/lib.rs index 15e38db..8889deb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,12 +80,14 @@ use raw_statement::RawStatement; pub use transaction::{SqliteTransaction, Transaction, TransactionBehavior}; pub use error::{SqliteError, Error}; +pub use cache::CachedStatement; #[cfg(feature = "load_extension")] pub use load_extension_guard::{SqliteLoadExtensionGuard, LoadExtensionGuard}; pub mod types; mod transaction; +mod cache; mod named_params; mod error; mod convenient; @@ -93,7 +95,6 @@ mod raw_statement; #[cfg(feature = "load_extension")]mod load_extension_guard; #[cfg(feature = "trace")]pub mod trace; #[cfg(feature = "backup")]pub mod backup; -#[cfg(feature = "cache")]mod cache; #[cfg(feature = "functions")]pub mod functions; #[cfg(feature = "blob")]pub mod blob; diff --git a/src/raw_statement.rs b/src/raw_statement.rs index 896508e..9676d94 100644 --- a/src/raw_statement.rs +++ b/src/raw_statement.rs @@ -44,7 +44,6 @@ impl RawStatement { } } - #[cfg(feature = "cache")] pub fn clear_bindings(&self) -> c_int { unsafe { ffi::sqlite3_clear_bindings(self.0) } } From 3c15eb0218d0e239f5095af1ba6ae9ea018303cc Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 12:11:25 -0500 Subject: [PATCH 30/39] Add Connection::prepare_cached. --- src/cache.rs | 76 +++++++++++++++++++++++++++++++++++++--------------- src/lib.rs | 7 +++++ 2 files changed, 61 insertions(+), 22 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 8d7ac06..452dcd3 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -6,6 +6,38 @@ use std::ops::{Deref, DerefMut}; use {Result, Connection, Statement}; use raw_statement::RawStatement; +impl Connection { + /// Prepare a SQL statement for execution, returning a previously prepared (but + /// not currently in-use) statement if one is available. The returned statement + /// will be cached for reuse by future calls to `prepare_cached` once it is + /// dropped. + /// + /// ```rust,no_run + /// # use rusqlite::{Connection, Result}; + /// fn insert_new_people(conn: &Connection) -> Result<()> { + /// { + /// let mut stmt = try!(conn.prepare_cached("INSERT INTO People (name) VALUES (?)")); + /// try!(stmt.execute(&[&"Joe Smith"])); + /// } + /// { + /// // This will return the same underlying SQLite statement handle without + /// // having to prepare it again. + /// let mut stmt = try!(conn.prepare_cached("INSERT INTO People (name) VALUES (?)")); + /// try!(stmt.execute(&[&"Bob Jones"])); + /// } + /// Ok(()) + /// } + /// ``` + /// + /// # Failure + /// + /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the + /// underlying SQLite call fails. + pub fn prepare_cached<'a>(&'a self, sql: &str) -> Result> { + self.cache.get(&self, sql) + } +} + /// Prepared statements LRU cache. #[derive(Debug)] pub struct StatementCache { @@ -93,21 +125,6 @@ impl StatementCache { stmt.clear_bindings(); cache.push_front(stmt) } - - /// Flush the prepared statement cache - pub fn clear(&self) { - self.cache.borrow_mut().clear(); - } - - /// Return current cache size. - pub fn len(&self) -> usize { - self.cache.borrow().len() - } - - /// Return maximum cache size. - pub fn capacity(&self) -> usize { - self.cache.borrow().capacity() - } } #[cfg(test)] @@ -115,16 +132,31 @@ mod test { use Connection; use super::StatementCache; + impl StatementCache { + fn clear(&self) { + self.cache.borrow_mut().clear(); + } + + fn len(&self) -> usize { + self.cache.borrow().len() + } + + fn capacity(&self) -> usize { + self.cache.borrow().capacity() + } + } + #[test] fn test_cache() { let db = Connection::open_in_memory().unwrap(); - let cache = StatementCache::with_capacity(15); + let cache = &db.cache; + let initial_capacity = cache.capacity(); assert_eq!(0, cache.len()); - assert_eq!(15, cache.capacity()); + assert!(initial_capacity > 0); let sql = "PRAGMA schema_version"; { - let mut stmt = cache.get(&db, sql).unwrap(); + let mut stmt = db.prepare_cached(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); @@ -132,7 +164,7 @@ mod test { assert_eq!(1, cache.len()); { - let mut stmt = cache.get(&db, sql).unwrap(); + let mut stmt = db.prepare_cached(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); @@ -141,17 +173,17 @@ mod test { cache.clear(); assert_eq!(0, cache.len()); - assert_eq!(15, cache.capacity()); + assert_eq!(initial_capacity, cache.capacity()); } #[test] fn test_discard() { let db = Connection::open_in_memory().unwrap(); - let cache = StatementCache::with_capacity(15); + let cache = &db.cache; let sql = "PRAGMA schema_version"; { - let mut stmt = cache.get(&db, sql).unwrap(); + let mut stmt = db.prepare_cached(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); diff --git a/src/lib.rs b/src/lib.rs index 8889deb..dbebe55 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -77,6 +77,7 @@ use libc::{c_int, c_char}; use types::{ToSql, FromSql}; use error::{error_from_sqlite_code, error_from_handle}; use raw_statement::RawStatement; +use cache::StatementCache; pub use transaction::{SqliteTransaction, Transaction, TransactionBehavior}; pub use error::{SqliteError, Error}; @@ -98,6 +99,9 @@ mod raw_statement; #[cfg(feature = "functions")]pub mod functions; #[cfg(feature = "blob")]pub mod blob; +// Number of cached prepared statements we'll hold on to. +const STATEMENT_CACHE_DEFAULT_CAPACITY: usize = 16; + /// Old name for `Result`. `SqliteResult` is deprecated. pub type SqliteResult = Result; @@ -150,6 +154,7 @@ pub type SqliteConnection = Connection; /// A connection to a SQLite database. pub struct Connection { db: RefCell, + cache: StatementCache, path: Option, } @@ -194,6 +199,7 @@ impl Connection { InnerConnection::open_with_flags(&c_path, flags).map(|db| { Connection { db: RefCell::new(db), + cache: StatementCache::with_capacity(STATEMENT_CACHE_DEFAULT_CAPACITY), path: Some(path.as_ref().to_path_buf()), } }) @@ -212,6 +218,7 @@ impl Connection { InnerConnection::open_with_flags(&c_memory, flags).map(|db| { Connection { db: RefCell::new(db), + cache: StatementCache::with_capacity(STATEMENT_CACHE_DEFAULT_CAPACITY), path: None, } }) From bd81b727f07547a1d9c982652b2d160293c8ea10 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 12:13:51 -0500 Subject: [PATCH 31/39] Simplify CachedStatement lifetimes --- src/cache.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 452dcd3..1d4bd51 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -33,7 +33,7 @@ impl Connection { /// /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the /// underlying SQLite call fails. - pub fn prepare_cached<'a>(&'a self, sql: &str) -> Result> { + pub fn prepare_cached<'a>(&'a self, sql: &str) -> Result> { self.cache.get(&self, sql) } } @@ -48,26 +48,26 @@ pub struct StatementCache { /// /// Statement will return automatically to the cache by default. /// If you want the statement to be discarded, call `discard()` on it. -pub struct CachedStatement<'c: 's, 's> { - stmt: Option>, - cache: &'s StatementCache, +pub struct CachedStatement<'conn> { + stmt: Option>, + cache: &'conn StatementCache, } -impl<'c, 's> Deref for CachedStatement<'c, 's> { - type Target = Statement<'c>; +impl<'conn> Deref for CachedStatement<'conn> { + type Target = Statement<'conn>; - fn deref(&self) -> &Statement<'c> { + fn deref(&self) -> &Statement<'conn> { self.stmt.as_ref().unwrap() } } -impl<'c, 's> DerefMut for CachedStatement<'c, 's> { - fn deref_mut(&mut self) -> &mut Statement<'c> { +impl<'conn> DerefMut for CachedStatement<'conn> { + fn deref_mut(&mut self) -> &mut Statement<'conn> { self.stmt.as_mut().unwrap() } } -impl<'c, 's> Drop for CachedStatement<'c, 's> { +impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { if let Some(stmt) = self.stmt.take() { @@ -76,8 +76,8 @@ impl<'c, 's> Drop for CachedStatement<'c, 's> { } } -impl<'c, 's> CachedStatement<'c, 's> { - fn new(stmt: Statement<'c>, cache: &'s StatementCache) -> CachedStatement<'c, 's> { +impl<'conn> CachedStatement<'conn> { + fn new(stmt: Statement<'conn>, cache: &'conn StatementCache) -> CachedStatement<'conn> { CachedStatement { stmt: Some(stmt), cache: cache, @@ -103,7 +103,7 @@ impl StatementCache { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get<'conn, 's>(&'s self, conn: &'conn Connection, sql: &str) -> Result> { + pub fn get<'conn>(&'conn self, conn: &'conn Connection, sql: &str) -> Result> { let mut cache = self.cache.borrow_mut(); let stmt = match cache.iter().rposition(|entry| entry.sql().to_bytes().eq(sql.as_bytes())) { Some(index) => { From 20b93bdb96fcd844a3523f75af5a26df4584c673 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 12:14:21 -0500 Subject: [PATCH 32/39] rustfmt --- src/cache.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 1d4bd51..a71be9c 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -92,9 +92,7 @@ impl<'conn> CachedStatement<'conn> { impl StatementCache { /// Create a statement cache. pub fn with_capacity(capacity: usize) -> StatementCache { - StatementCache { - cache: RefCell::new(VecDeque::with_capacity(capacity)), - } + StatementCache { cache: RefCell::new(VecDeque::with_capacity(capacity)) } } /// Search the cache for a prepared-statement object that implements `sql`. @@ -102,10 +100,15 @@ impl StatementCache { /// /// # Failure /// - /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get<'conn>(&'conn self, conn: &'conn Connection, sql: &str) -> Result> { + /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare + /// call fails. + pub fn get<'conn>(&'conn self, + conn: &'conn Connection, + sql: &str) + -> Result> { let mut cache = self.cache.borrow_mut(); - let stmt = match cache.iter().rposition(|entry| entry.sql().to_bytes().eq(sql.as_bytes())) { + let stmt = match cache.iter() + .rposition(|entry| entry.sql().to_bytes().eq(sql.as_bytes())) { Some(index) => { let raw_stmt = cache.swap_remove_front(index).unwrap(); // FIXME Not LRU compliant Ok(Statement::new(conn, raw_stmt)) @@ -159,7 +162,7 @@ mod test { let mut stmt = db.prepare_cached(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); } assert_eq!(1, cache.len()); @@ -167,7 +170,7 @@ mod test { let mut stmt = db.prepare_cached(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); } assert_eq!(1, cache.len()); @@ -186,7 +189,7 @@ mod test { let mut stmt = db.prepare_cached(sql).unwrap(); assert_eq!(0, cache.len()); assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); stmt.discard(); } assert_eq!(0, cache.len()); From d923d8c670e5cbc5d7b833a4d16ceeb4c0e0038f Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 12:26:17 -0500 Subject: [PATCH 33/39] Use a real LruCache instead of a VecDeque. --- Cargo.toml | 1 + src/cache.rs | 35 +++++++++++++---------------------- src/lib.rs | 1 + 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ccabe9d..3babbc3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ trace = [] [dependencies] time = "~0.1.0" bitflags = "0.7" +lru-cache = "0.0.7" libc = "~0.2" clippy = {version = "~0.0.58", optional = true} chrono = { version = "~0.2", optional = true } diff --git a/src/cache.rs b/src/cache.rs index a71be9c..e9dfd1a 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,8 +1,8 @@ //! Prepared statements cache for faster execution. use std::cell::RefCell; -use std::collections::VecDeque; use std::ops::{Deref, DerefMut}; +use lru_cache::LruCache; use {Result, Connection, Statement}; use raw_statement::RawStatement; @@ -40,9 +40,7 @@ impl Connection { /// Prepared statements LRU cache. #[derive(Debug)] -pub struct StatementCache { - cache: RefCell>, // back = LRU -} +pub struct StatementCache(RefCell>); /// Cacheable statement. /// @@ -92,7 +90,7 @@ impl<'conn> CachedStatement<'conn> { impl StatementCache { /// Create a statement cache. pub fn with_capacity(capacity: usize) -> StatementCache { - StatementCache { cache: RefCell::new(VecDeque::with_capacity(capacity)) } + StatementCache(RefCell::new(LruCache::new(capacity))) } /// Search the cache for a prepared-statement object that implements `sql`. @@ -106,27 +104,20 @@ impl StatementCache { conn: &'conn Connection, sql: &str) -> Result> { - let mut cache = self.cache.borrow_mut(); - let stmt = match cache.iter() - .rposition(|entry| entry.sql().to_bytes().eq(sql.as_bytes())) { - Some(index) => { - let raw_stmt = cache.swap_remove_front(index).unwrap(); // FIXME Not LRU compliant - Ok(Statement::new(conn, raw_stmt)) - } - _ => conn.prepare(sql), + let mut cache = self.0.borrow_mut(); + let stmt = match cache.remove(sql) { + Some(raw_stmt) => Ok(Statement::new(conn, raw_stmt)), + None => conn.prepare(sql), }; stmt.map(|stmt| CachedStatement::new(stmt, self)) } // Return a statement to the cache. fn cache_stmt(&self, stmt: RawStatement) { - let mut cache = self.cache.borrow_mut(); - if cache.capacity() == cache.len() { - // is full - cache.pop_back(); // LRU dropped - } + let mut cache = self.0.borrow_mut(); stmt.clear_bindings(); - cache.push_front(stmt) + let sql = String::from_utf8_lossy(stmt.sql().to_bytes()).to_string(); + cache.insert(sql, stmt); } } @@ -137,15 +128,15 @@ mod test { impl StatementCache { fn clear(&self) { - self.cache.borrow_mut().clear(); + self.0.borrow_mut().clear(); } fn len(&self) -> usize { - self.cache.borrow().len() + self.0.borrow().len() } fn capacity(&self) -> usize { - self.cache.borrow().capacity() + self.0.borrow().capacity() } } diff --git a/src/lib.rs b/src/lib.rs index dbebe55..eea21cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,6 +55,7 @@ extern crate libc; extern crate libsqlite3_sys as ffi; +extern crate lru_cache; #[macro_use] extern crate bitflags; #[cfg(test)] From e71c3c5207d5f9762cec74824bf3b3ea3d162fdb Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 14:20:56 -0500 Subject: [PATCH 34/39] Add Connection::set_prepared_statement_cache_capacity. --- src/cache.rs | 63 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index e9dfd1a..b39b494 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -36,6 +36,14 @@ impl Connection { pub fn prepare_cached<'a>(&'a self, sql: &str) -> Result> { self.cache.get(&self, sql) } + + /// Set the maximum number of cached prepared statements this connection will hold. + /// By default, a connection will hold a relatively small number of cached statements. + /// If you need more, or know that you will not use cached statements, you can set + /// the capacity manually using this method. + pub fn set_prepared_statement_cache_capacity(&self, capacity: usize) { + self.cache.set_capacity(capacity) + } } /// Prepared statements LRU cache. @@ -93,14 +101,18 @@ impl StatementCache { StatementCache(RefCell::new(LruCache::new(capacity))) } - /// Search the cache for a prepared-statement object that implements `sql`. - /// If no such prepared-statement can be found, allocate and prepare a new one. - /// - /// # Failure - /// - /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare - /// call fails. - pub fn get<'conn>(&'conn self, + fn set_capacity(&self, capacity: usize) { + self.0.borrow_mut().set_capacity(capacity) + } + + // Search the cache for a prepared-statement object that implements `sql`. + // If no such prepared-statement can be found, allocate and prepare a new one. + // + // # Failure + // + // Will return `Err` if no cached statement can be found and the underlying SQLite prepare + // call fails. + fn get<'conn>(&'conn self, conn: &'conn Connection, sql: &str) -> Result> { @@ -170,6 +182,41 @@ mod test { assert_eq!(initial_capacity, cache.capacity()); } + #[test] + fn test_set_capacity() { + let db = Connection::open_in_memory().unwrap(); + let cache = &db.cache; + + let sql = "PRAGMA schema_version"; + { + let mut stmt = db.prepare_cached(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + } + assert_eq!(1, cache.len()); + + db.set_prepared_statement_cache_capacity(0); + assert_eq!(0, cache.len()); + + { + let mut stmt = db.prepare_cached(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + } + assert_eq!(0, cache.len()); + + db.set_prepared_statement_cache_capacity(8); + { + let mut stmt = db.prepare_cached(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + } + assert_eq!(1, cache.len()); + } + #[test] fn test_discard() { let db = Connection::open_in_memory().unwrap(); From e4e17cc5ddfe3bd1f895c62b4f639e577563b122 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Tue, 17 May 2016 15:47:07 -0500 Subject: [PATCH 35/39] Update changelog description of statement caching. --- Changelog.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index 72ec1bf..76cb1d6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,7 +1,9 @@ # Version UPCOMING (...) -* Adds a `cache` Cargo feature that provides `cache::StatementCache` for caching prepared - statements. +* 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 `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. From 678b301494ffc4226b7e4c76f8ddd524a24e76ed Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 18 May 2016 14:25:57 -0500 Subject: [PATCH 36/39] Make rollback take (&mut self) instead of (self) --- src/transaction.rs | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/transaction.rs b/src/transaction.rs index a30fc30..79bb7d9 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -138,13 +138,8 @@ impl<'conn> Transaction<'conn> { self.conn.execute_batch(&sql) } - /// A convenience method which consumes and rolls back a transaction. - pub fn rollback(mut self) -> Result<()> { - self.rollback_() - } - - fn rollback_(&mut self) -> Result<()> { - self.finished = true; + /// A convenience method which rolls back a transaction. + pub fn rollback(&mut self) -> Result<()> { let sql = if self.depth == 0 { Cow::Borrowed("ROLLBACK") } else { @@ -166,7 +161,7 @@ impl<'conn> Transaction<'conn> { match (self.finished, self.commit) { (true, _) => Ok(()), (false, true) => self.commit_(), - (false, false) => self.rollback_(), + (false, false) => self.rollback(), } } } @@ -221,18 +216,24 @@ mod test { fn test_explicit_rollback_commit() { let mut db = checked_memory_handle(); { - let tx = db.transaction().unwrap(); - tx.execute_batch("INSERT INTO foo VALUES(1)").unwrap(); - tx.rollback().unwrap(); - } - { - let tx = db.transaction().unwrap(); - tx.execute_batch("INSERT INTO foo VALUES(2)").unwrap(); + let mut tx = db.transaction().unwrap(); + { + let mut sp = tx.savepoint().unwrap(); + sp.execute_batch("INSERT INTO foo VALUES(1)").unwrap(); + sp.rollback().unwrap(); + sp.execute_batch("INSERT INTO foo VALUES(2)").unwrap(); + sp.commit().unwrap(); + } tx.commit().unwrap(); } { let tx = db.transaction().unwrap(); - assert_eq!(2i32, + tx.execute_batch("INSERT INTO foo VALUES(4)").unwrap(); + tx.commit().unwrap(); + } + { + let tx = db.transaction().unwrap(); + assert_eq!(6i32, tx.query_row("SELECT SUM(x) FROM foo", &[], |r| r.get(0)).unwrap()); } } From 703cf22b52b4a287c8e9d1f7a7990cc183597153 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 18 May 2016 16:38:09 -0500 Subject: [PATCH 37/39] Separate Savepoint out from Transaction. Replaces `set_commit` and `set_rollback` with `set_drop_behavior`. Allows specification of savepoint names. Savepoint::rollback() does not consume the savepoint; Transaction::rollback() does consume the transaction. --- src/lib.rs | 39 ----- src/transaction.rs | 388 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 328 insertions(+), 99 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8708869..69946d6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -213,45 +213,6 @@ impl Connection { }) } - /// Begin a new transaction with the default behavior (DEFERRED). - /// - /// The transaction defaults to rolling back when it is dropped. If you want the transaction to - /// commit, you must call `commit` or `set_commit`. - /// - /// ## Example - /// - /// ```rust,no_run - /// # use rusqlite::{Connection, Result}; - /// # fn do_queries_part_1(conn: &Connection) -> Result<()> { Ok(()) } - /// # fn do_queries_part_2(conn: &Connection) -> Result<()> { Ok(()) } - /// fn perform_queries(conn: &Connection) -> Result<()> { - /// let tx = try!(conn.transaction()); - /// - /// try!(do_queries_part_1(conn)); // tx causes rollback if this fails - /// try!(do_queries_part_2(conn)); // tx causes rollback if this fails - /// - /// tx.commit() - /// } - /// ``` - /// - /// # Failure - /// - /// Will return `Err` if the underlying SQLite call fails. - pub fn transaction(&mut self) -> Result { - Transaction::new(self, TransactionBehavior::Deferred) - } - - /// Begin a new transaction with a specified behavior. - /// - /// See `transaction`. - /// - /// # Failure - /// - /// Will return `Err` if the underlying SQLite call fails. - pub fn transaction_with_behavior(&mut self, behavior: TransactionBehavior) -> Result { - Transaction::new(self, behavior) - } - /// Convenience method to run multiple SQL statements (that cannot take any parameters). /// /// Uses [sqlite3_exec](http://www.sqlite.org/c3ref/exec.html) under the hood. diff --git a/src/transaction.rs b/src/transaction.rs index 79bb7d9..97caa2e 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow; use std::ops::Deref; use {Result, Connection}; @@ -14,16 +13,30 @@ pub enum TransactionBehavior { Exclusive, } +/// Options for how a Transaction or Savepoint should behave when it is dropped. +#[derive(Copy,Clone,PartialEq,Eq)] +pub enum DropBehavior { + /// Roll back the changes. This is the default. + Rollback, + + /// Commit the changes. + Commit, + + /// Do not commit or roll back changes - this will leave the transaction or savepoint + /// open, so should be used with care. + Ignore, +} + /// Old name for `Transaction`. `SqliteTransaction` is deprecated. pub type SqliteTransaction<'conn> = Transaction<'conn>; -/// /// Represents a transaction on a database connection. /// /// ## Note /// -/// Transactions will roll back by default. Use the `set_commit` or `commit` methods to commit the -/// transaction. +/// Transactions will roll back by default. Use `commit` method to explicitly commit the +/// transaction, or use `set_drop_behavior` to change what happens when the transaction +/// is dropped. /// /// ## Example /// @@ -42,9 +55,39 @@ pub type SqliteTransaction<'conn> = Transaction<'conn>; /// ``` pub struct Transaction<'conn> { conn: &'conn Connection, + drop_behavior: DropBehavior, + committed: bool, +} + +/// Represents a savepoint on a database connection. +/// +/// ## Note +/// +/// Savepoints will roll back by default. Use `commit` method to explicitly commit the +/// savepoint, or use `set_drop_behavior` to change what happens when the savepoint +/// is dropped. +/// +/// ## Example +/// +/// ```rust,no_run +/// # use rusqlite::{Connection, Result}; +/// # fn do_queries_part_1(conn: &Connection) -> Result<()> { Ok(()) } +/// # fn do_queries_part_2(conn: &Connection) -> Result<()> { Ok(()) } +/// fn perform_queries(conn: &Connection) -> Result<()> { +/// let sp = try!(conn.savepoint()); +/// +/// try!(do_queries_part_1(conn)); // sp causes rollback if this fails +/// try!(do_queries_part_2(conn)); // sp causes rollback if this fails +/// +/// sp.commit() +/// } +/// ``` +pub struct Savepoint<'conn> { + conn: &'conn Connection, + name: String, depth: u32, - commit: bool, - finished: bool, + drop_behavior: DropBehavior, + committed: bool, } impl<'conn> Transaction<'conn> { @@ -58,9 +101,8 @@ impl<'conn> Transaction<'conn> { conn.execute_batch(query).map(move |_| { Transaction { conn: conn, - depth: 0, - commit: false, - finished: false, + drop_behavior: DropBehavior::Rollback, + committed: false, } }) } @@ -91,36 +133,23 @@ impl<'conn> Transaction<'conn> { /// tx.commit() /// } /// ``` - pub fn savepoint(&mut self) -> Result { - let new_depth = self.depth + 1; - self.conn.execute_batch(&format!("SAVEPOINT sp{}", new_depth)).map(|_| { - Transaction { - conn: self.conn, - depth: new_depth, - commit: false, - finished: false, - } - }) + pub fn savepoint(&mut self) -> Result { + Savepoint::with_depth(self.conn, 1) } - /// Returns whether or not the transaction is currently set to commit. - pub fn will_commit(&self) -> bool { - self.commit + /// Create a new savepoint with a custom savepoint name. See `savepoint()`. + pub fn savepoint_with_name>(&mut self, name: T) -> Result { + Savepoint::with_depth_and_name(self.conn, 1, name) } - /// Returns whether or not the transaction is currently set to rollback. - pub fn will_rollback(&self) -> bool { - !self.commit + /// Get the current setting for what happens to the transaction when it is dropped. + pub fn drop_behavior(&self) -> DropBehavior { + self.drop_behavior } - /// Set the transaction to commit at its completion. - pub fn set_commit(&mut self) { - self.commit = true - } - - /// Set the transaction to rollback at its completion. - pub fn set_rollback(&mut self) { - self.commit = false + /// Configure the transaction to perform the specified action when it is dropped. + pub fn set_drop_behavior(&mut self, drop_behavior: DropBehavior) { + self.drop_behavior = drop_behavior } /// A convenience method which consumes and commits a transaction. @@ -129,27 +158,22 @@ impl<'conn> Transaction<'conn> { } fn commit_(&mut self) -> Result<()> { - self.finished = true; - let sql = if self.depth == 0 { - Cow::Borrowed("COMMIT") - } else { - Cow::Owned(format!("RELEASE sp{}", self.depth)) - }; - self.conn.execute_batch(&sql) + self.committed = true; + self.conn.execute_batch("COMMIT") } - /// A convenience method which rolls back a transaction. - pub fn rollback(&mut self) -> Result<()> { - let sql = if self.depth == 0 { - Cow::Borrowed("ROLLBACK") - } else { - Cow::Owned(format!("ROLLBACK TO sp{}", self.depth)) - }; - self.conn.execute_batch(&sql) + /// A convenience method which consumes and rolls back a transaction. + pub fn rollback(mut self) -> Result<()> { + self.rollback_() + } + + fn rollback_(&mut self) -> Result<()> { + self.committed = true; + self.conn.execute_batch("ROLLBACK") } /// Consumes the transaction, committing or rolling back according to the current setting - /// (see `will_commit`, `will_rollback`). + /// (see `drop_behavior`). /// /// Functionally equivalent to the `Drop` implementation, but allows callers to see any /// errors that occur. @@ -158,10 +182,13 @@ impl<'conn> Transaction<'conn> { } fn finish_(&mut self) -> Result<()> { - match (self.finished, self.commit) { - (true, _) => Ok(()), - (false, true) => self.commit_(), - (false, false) => self.rollback(), + if self.committed { + return Ok(()); + } + match self.drop_behavior() { + DropBehavior::Commit => self.commit_(), + DropBehavior::Rollback => self.rollback_(), + DropBehavior::Ignore => Ok(()), } } } @@ -181,10 +208,196 @@ impl<'conn> Drop for Transaction<'conn> { } } +impl<'conn> Savepoint<'conn> { + fn with_depth_and_name>(conn: &Connection, depth: u32, name: T) -> Result { + let name = name.into(); + conn.execute_batch(&format!("SAVEPOINT {}", name)).map(|_| { + Savepoint { + conn: conn, + name: name, + depth: depth, + drop_behavior: DropBehavior::Rollback, + committed: false, + } + }) + } + + fn with_depth(conn: &Connection, depth: u32) -> Result { + let name = format!("_rusqlite_sp_{}", depth); + Savepoint::with_depth_and_name(conn, depth, name) + } + + /// Begin a new savepoint. Can be nested. + pub fn new(conn: &mut Connection) -> Result { + Savepoint::with_depth(conn, 0) + } + + /// Begin a new savepoint with a user-provided savepoint name. + pub fn with_name>(conn: &mut Connection, name: T) -> Result { + Savepoint::with_depth_and_name(conn, 0, name) + } + + /// Begin a nested savepoint. + pub fn savepoint(&mut self) -> Result { + Savepoint::with_depth(self.conn, self.depth + 1) + } + + /// Begin a nested savepoint with a user-provided savepoint name. + pub fn savepoint_with_name>(&mut self, name: T) -> Result { + Savepoint::with_depth_and_name(self.conn, self.depth + 1, name) + } + + /// Get the current setting for what happens to the savepoint when it is dropped. + pub fn drop_behavior(&self) -> DropBehavior { + self.drop_behavior + } + + /// Configure the savepoint to perform the specified action when it is dropped. + pub fn set_drop_behavior(&mut self, drop_behavior: DropBehavior) { + self.drop_behavior = drop_behavior + } + + /// A convenience method which consumes and commits a savepoint. + pub fn commit(mut self) -> Result<()> { + self.commit_() + } + + fn commit_(&mut self) -> Result<()> { + self.committed = true; + self.conn.execute_batch(&format!("RELEASE {}", self.name)) + } + + /// A convenience method which rolls back a savepoint. + /// + /// ## Note + /// + /// Unlike `Transaction`s, savepoints remain active after they have been rolled back, + /// and can be rolled back again or committed. + pub fn rollback(&mut self) -> Result<()> { + self.conn.execute_batch(&format!("ROLLBACK TO {}", self.name)) + } + + /// Consumes the savepoint, committing or rolling back according to the current setting + /// (see `drop_behavior`). + /// + /// Functionally equivalent to the `Drop` implementation, but allows callers to see any + /// errors that occur. + pub fn finish(mut self) -> Result<()> { + self.finish_() + } + + fn finish_(&mut self) -> Result<()> { + if self.committed { + return Ok(()); + } + match self.drop_behavior() { + DropBehavior::Commit => self.commit_(), + DropBehavior::Rollback => self.rollback(), + DropBehavior::Ignore => Ok(()), + } + } +} + +impl<'conn> Deref for Savepoint<'conn> { + type Target = Connection; + + fn deref(&self) -> &Connection { + self.conn + } +} + +#[allow(unused_must_use)] +impl<'conn> Drop for Savepoint<'conn> { + fn drop(&mut self) { + self.finish_(); + } +} + +impl Connection { + /// Begin a new transaction with the default behavior (DEFERRED). + /// + /// The transaction defaults to rolling back when it is dropped. If you want the transaction to + /// commit, you must call `commit` or `set_drop_behavior(DropBehavior::Commit)`. + /// + /// ## Example + /// + /// ```rust,no_run + /// # use rusqlite::{Connection, Result}; + /// # fn do_queries_part_1(conn: &Connection) -> Result<()> { Ok(()) } + /// # fn do_queries_part_2(conn: &Connection) -> Result<()> { Ok(()) } + /// fn perform_queries(conn: &Connection) -> Result<()> { + /// let tx = try!(conn.transaction()); + /// + /// try!(do_queries_part_1(conn)); // tx causes rollback if this fails + /// try!(do_queries_part_2(conn)); // tx causes rollback if this fails + /// + /// tx.commit() + /// } + /// ``` + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. + pub fn transaction(&mut self) -> Result { + Transaction::new(self, TransactionBehavior::Deferred) + } + + /// Begin a new transaction with a specified behavior. + /// + /// See `transaction`. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. + pub fn transaction_with_behavior(&mut self, behavior: TransactionBehavior) -> Result { + Transaction::new(self, behavior) + } + + /// Begin a new savepoint with the default behavior (DEFERRED). + /// + /// The savepoint defaults to rolling back when it is dropped. If you want the savepoint to + /// commit, you must call `commit` or `set_drop_behavior(DropBehavior::Commit)`. + /// + /// ## Example + /// + /// ```rust,no_run + /// # use rusqlite::{Connection, Result}; + /// # fn do_queries_part_1(conn: &Connection) -> Result<()> { Ok(()) } + /// # fn do_queries_part_2(conn: &Connection) -> Result<()> { Ok(()) } + /// fn perform_queries(conn: &Connection) -> Result<()> { + /// let sp = try!(conn.savepoint()); + /// + /// try!(do_queries_part_1(conn)); // sp causes rollback if this fails + /// try!(do_queries_part_2(conn)); // sp causes rollback if this fails + /// + /// sp.commit() + /// } + /// ``` + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. + pub fn savepoint(&mut self) -> Result { + Savepoint::new(self) + } + + /// Begin a new savepoint with a specified name. + /// + /// See `savepoint`. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. + pub fn savepoint_with_name>(&mut self, name: T) -> Result { + Savepoint::with_name(self, name) + } +} + #[cfg(test)] #[cfg_attr(feature="clippy", allow(similar_names))] mod test { use Connection; + use super::DropBehavior; fn checked_memory_handle() -> Connection { let db = Connection::open_in_memory().unwrap(); @@ -203,7 +416,7 @@ mod test { { let mut tx = db.transaction().unwrap(); tx.execute_batch("INSERT INTO foo VALUES(2)").unwrap(); - tx.set_commit() + tx.set_drop_behavior(DropBehavior::Commit) } { let tx = db.transaction().unwrap(); @@ -240,17 +453,12 @@ mod test { #[test] fn test_savepoint() { - fn assert_current_sum(x: i32, conn: &Connection) { - let i = conn.query_row("SELECT SUM(x) FROM foo", &[], |r| r.get(0)).unwrap(); - assert_eq!(x, i); - } - let mut db = checked_memory_handle(); { let mut tx = db.transaction().unwrap(); tx.execute_batch("INSERT INTO foo VALUES(1)").unwrap(); assert_current_sum(1, &tx); - tx.set_commit(); + tx.set_drop_behavior(DropBehavior::Commit); { let mut sp1 = tx.savepoint().unwrap(); sp1.execute_batch("INSERT INTO foo VALUES(2)").unwrap(); @@ -276,4 +484,64 @@ mod test { } assert_current_sum(1, &db); } + + #[test] + fn test_ignore_drop_behavior() { + let mut db = checked_memory_handle(); + + let mut tx = db.transaction().unwrap(); + { + let mut sp1 = tx.savepoint().unwrap(); + insert(1, &sp1); + sp1.rollback().unwrap(); + insert(2, &sp1); + { + let mut sp2 = sp1.savepoint().unwrap(); + sp2.set_drop_behavior(DropBehavior::Ignore); + insert(4, &sp2); + } + assert_current_sum(6, &sp1); + sp1.commit().unwrap(); + } + assert_current_sum(6, &tx); + } + + #[test] + fn test_savepoint_names() { + let mut db = checked_memory_handle(); + + { + let mut sp1 = db.savepoint_with_name("my_sp").unwrap(); + insert(1, &sp1); + assert_current_sum(1, &sp1); + { + let mut sp2 = sp1.savepoint_with_name("my_sp").unwrap(); + sp2.set_drop_behavior(DropBehavior::Commit); + insert(2, &sp2); + assert_current_sum(3, &sp2); + sp2.rollback().unwrap(); + assert_current_sum(1, &sp2); + insert(4, &sp2); + } + assert_current_sum(5, &sp1); + sp1.rollback().unwrap(); + { + let mut sp2 = sp1.savepoint_with_name("my_sp").unwrap(); + sp2.set_drop_behavior(DropBehavior::Ignore); + insert(8, &sp2); + } + assert_current_sum(8, &sp1); + sp1.commit().unwrap(); + } + assert_current_sum(8, &db); + } + + fn insert(x: i32, conn: &Connection) { + conn.execute("INSERT INTO foo VALUES(?)", &[&x]).unwrap(); + } + + fn assert_current_sum(x: i32, conn: &Connection) { + let i = conn.query_row("SELECT SUM(x) FROM foo", &[], |r| r.get(0)).unwrap(); + assert_eq!(x, i); + } } From a19807b8e40bf15d13d4187a8405f0dd36e42ce5 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 18 May 2016 16:40:38 -0500 Subject: [PATCH 38/39] Update Changelog with more Transaction changes. --- Changelog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Changelog.md b/Changelog.md index 282c805..26a2b58 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,11 @@ connection are inherently nested. While a transaction is alive, the parent connection or transaction is unusable, so `Transaction` now implements `Deref`, giving 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 `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. From 74b57ee47ac343e5b29c09d31c6664d6ca27005e Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 18 May 2016 22:19:04 -0500 Subject: [PATCH 39/39] Add test and fix for invalid cached column_count. Issue raised in https://github.com/jgallagher/rusqlite/pull/113#issuecomment-220122048. --- src/cache.rs | 40 +++++++++++++++++++++++++++++++++++++--- src/lib.rs | 13 +++++-------- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index b39b494..f5fade3 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -113,9 +113,9 @@ impl StatementCache { // Will return `Err` if no cached statement can be found and the underlying SQLite prepare // call fails. fn get<'conn>(&'conn self, - conn: &'conn Connection, - sql: &str) - -> Result> { + conn: &'conn Connection, + sql: &str) + -> Result> { let mut cache = self.0.borrow_mut(); let stmt = match cache.remove(sql) { Some(raw_stmt) => Ok(Statement::new(conn, raw_stmt)), @@ -232,4 +232,38 @@ mod test { } assert_eq!(0, cache.len()); } + + #[test] + fn test_ddl() { + let db = Connection::open_in_memory().unwrap(); + db.execute_batch(r#" + CREATE TABLE foo (x INT); + INSERT INTO foo VALUES (1); + "#) + .unwrap(); + + let sql = "SELECT * FROM foo"; + + { + let mut stmt = db.prepare_cached(sql).unwrap(); + assert_eq!(1i32, + stmt.query_map(&[], |r| r.get(0)).unwrap().next().unwrap().unwrap()); + } + + db.execute_batch(r#" + ALTER TABLE foo ADD COLUMN y INT; + UPDATE foo SET y = 2; + "#) + .unwrap(); + + { + let mut stmt = db.prepare_cached(sql).unwrap(); + assert_eq!((1i32, 2i32), + stmt.query_map(&[], |r| (r.get(0), r.get(1))) + .unwrap() + .next() + .unwrap() + .unwrap()); + } + } } diff --git a/src/lib.rs b/src/lib.rs index 130ee2d..bb45e1c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -720,22 +720,19 @@ pub type SqliteStatement<'conn> = Statement<'conn>; pub struct Statement<'conn> { conn: &'conn Connection, stmt: RawStatement, - column_count: c_int, } impl<'conn> Statement<'conn> { fn new(conn: &Connection, stmt: RawStatement) -> Statement { - let column_count = stmt.column_count(); Statement { conn: conn, stmt: stmt, - column_count: column_count, } } /// Get all the column names in the result set of the prepared statement. pub fn column_names(&self) -> Vec<&str> { - let n = self.column_count; + let n = self.column_count(); let mut cols = Vec::with_capacity(n as usize); for i in 0..n { let slice = self.stmt.column_name(i); @@ -747,7 +744,7 @@ impl<'conn> Statement<'conn> { /// Return the number of columns in the result set returned by the prepared statement. pub fn column_count(&self) -> i32 { - self.column_count + self.stmt.column_count() } /// Returns the column index in the result set for a given column name. @@ -757,7 +754,7 @@ impl<'conn> Statement<'conn> { /// Will return an `Error::InvalidColumnName` when there is no column with the specified `name`. pub fn column_index(&self, name: &str) -> Result { let bytes = name.as_bytes(); - let n = self.column_count; + let n = self.column_count(); for i in 0..n { if bytes == self.stmt.column_name(i).to_bytes() { return Ok(i); @@ -799,7 +796,7 @@ impl<'conn> Statement<'conn> { self.stmt.reset(); match r { ffi::SQLITE_DONE => { - if self.column_count == 0 { + if self.column_count() == 0 { Ok(self.conn.changes()) } else { Err(Error::ExecuteReturnedResults) @@ -1166,7 +1163,7 @@ pub trait RowIndex { impl RowIndex for i32 { #[inline] fn idx(&self, stmt: &Statement) -> Result { - if *self < 0 || *self >= stmt.column_count { + if *self < 0 || *self >= stmt.column_count() { Err(Error::InvalidColumnIndex(*self)) } else { Ok(*self)