From 9c00dd01a2b7ed3bc50466e2b5bedbbb8878a0b4 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 3 Feb 2019 09:17:37 +0100 Subject: [PATCH 01/15] Draft to ease PRAGMA usage (#273 and #265) --- src/lib.rs | 1 + src/pragma.rs | 405 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 406 insertions(+) create mode 100644 src/pragma.rs diff --git a/src/lib.rs b/src/lib.rs index 9bb11a9..03318ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -117,6 +117,7 @@ mod inner_connection; pub mod limits; #[cfg(feature = "load_extension")] mod load_extension_guard; +mod pragma; mod raw_statement; mod row; #[cfg(feature = "session")] diff --git a/src/pragma.rs b/src/pragma.rs new file mode 100644 index 0000000..a9daff8 --- /dev/null +++ b/src/pragma.rs @@ -0,0 +1,405 @@ +//! Pragma helpers + +use std::ops::Deref; + +use crate::error::Error; +use crate::ffi; +use crate::types::{ToSql, ToSqlOutput, ValueRef}; +use crate::{Connection, DatabaseName, Result, Row, NO_PARAMS}; + +pub struct Sql { + buf: String, +} + +impl Sql { + pub fn new() -> Sql { + Sql { buf: String::new() } + } + + pub fn push_pragma( + &mut self, + schema_name: Option>, + pragma_name: &str, + ) -> Result<()> { + self.push_keyword("PRAGMA")?; + self.push_space(); + if let Some(schema_name) = schema_name { + self.push_schema_name(schema_name); + self.push_dot(); + } + self.push_keyword(pragma_name) + } + + pub fn push_keyword(&mut self, keyword: &str) -> Result<()> { + if !keyword.is_empty() && is_identifier(keyword) { + self.buf.push_str(keyword); + Ok(()) + } else { + Err(Error::SqliteFailure( + ffi::Error::new(ffi::SQLITE_MISUSE), + Some(format!("Invalid keyword \"{}\"", keyword)), + )) + } + } + + pub fn push_schema_name(&mut self, schema_name: DatabaseName<'_>) { + match schema_name { + DatabaseName::Main => self.buf.push_str("main"), + DatabaseName::Temp => self.buf.push_str("temp"), + DatabaseName::Attached(s) => self.push_identifier(s), + }; + } + + pub fn push_identifier(&mut self, s: &str) { + if is_identifier(s) { + self.buf.push_str(s); + } else { + self.wrap_and_escape(s, '"'); + } + } + + pub fn push_value(&mut self, value: &dyn ToSql) -> Result<()> { + let value = value.to_sql()?; + let value = match value { + ToSqlOutput::Borrowed(v) => v, + ToSqlOutput::Owned(ref v) => ValueRef::from(v), + #[cfg(feature = "blob")] + ToSqlOutput::ZeroBlob(_) => { + return Err(Error::SqliteFailure( + ffi::Error::new(ffi::SQLITE_MISUSE), + Some(format!("Unsupported value \"{:?}\"", value)), + )); + } + #[cfg(feature = "array")] + ToSqlOutput::Array(_) => { + return Err(Error::SqliteFailure( + ffi::Error::new(ffi::SQLITE_MISUSE), + Some(format!("Unsupported value \"{:?}\"", value)), + )); + } + }; + match value { + ValueRef::Integer(i) => { + self.push_int(i); + } + ValueRef::Real(r) => { + self.push_real(r); + } + ValueRef::Text(s) => { + self.push_string_literal(s); + } + _ => { + return Err(Error::SqliteFailure( + ffi::Error::new(ffi::SQLITE_MISUSE), + Some(format!("Unsupported value \"{:?}\"", value)), + )); + } + }; + Ok(()) + } + + pub fn push_string_literal(&mut self, s: &str) { + self.wrap_and_escape(s, '\''); + } + + pub fn push_int(&mut self, i: i64) { + self.buf.push_str(&i.to_string()); + } + + pub fn push_real(&mut self, f: f64) { + self.buf.push_str(&f.to_string()); + } + + pub fn push_space(&mut self) { + self.buf.push(' '); + } + + pub fn push_dot(&mut self) { + self.buf.push('.'); + } + + pub fn push_equal_sign(&mut self) { + self.buf.push('='); + } + + pub fn open_brace(&mut self) { + self.buf.push('('); + } + + pub fn close_brace(&mut self) { + self.buf.push(')'); + } + + pub fn as_str(&self) -> &str { + &self.buf + } + + fn wrap_and_escape(&mut self, s: &str, quote: char) { + self.buf.push(quote); + let chars = s.chars(); + for ch in chars { + // escape `quote` by doubling it + if ch == quote { + self.buf.push(ch); + } + self.buf.push(ch) + } + self.buf.push(quote); + } +} + +impl Deref for Sql { + type Target = str; + + fn deref(&self) -> &str { + self.as_str() + } +} + +impl Connection { + /// Query the current value of `pragma_name`. + /// + /// Some pragmas will return multiple rows/values which cannot be retrieved + /// with this method. + pub fn pragma_query_value( + &self, + schema_name: Option>, + pragma_name: &str, + f: F, + ) -> Result + where + F: FnOnce(Row<'_, '_>) -> Result, + { + let mut query = Sql::new(); + query.push_pragma(schema_name, pragma_name)?; + let mut stmt = self.prepare(&query)?; + let mut rows = stmt.query(NO_PARAMS)?; + if let Some(result_row) = rows.next() { + let row = result_row?; + f(row) + } else { + Err(Error::QueryReturnedNoRows) + } + } + + /// Query the current rows/values of `pragma_name`. + pub fn pragma_query( + &self, + schema_name: Option>, + pragma_name: &str, + mut f: F, + ) -> Result<()> + where + F: FnMut(&Row<'_, '_>) -> Result<()>, + { + let mut query = Sql::new(); + query.push_pragma(schema_name, pragma_name)?; + let mut stmt = self.prepare(&query)?; + let mut rows = stmt.query(NO_PARAMS)?; + while let Some(result_row) = rows.next() { + let row = result_row?; + f(&row)?; + } + Ok(()) + } + + /// Query the current value(s) of `pragma_name` associated to + /// `pragma_value`. + /// + /// This method can be used with query-only pragmas which need an argument + /// (e.g. `table_info('one_tbl')`) or pragmas which return the updated value + /// (e.g. `journal_mode`). + pub fn pragma( + &self, + schema_name: Option>, + pragma_name: &str, + pragma_value: &dyn ToSql, + mut f: F, + ) -> Result<()> + where + F: FnMut(&Row<'_, '_>) -> Result<()>, + { + let mut sql = Sql::new(); + sql.push_pragma(schema_name, pragma_name)?; + // The argument is may be either in parentheses + // or it may be separated from the pragma name by an equal sign. + // The two syntaxes yield identical results. + sql.open_brace(); + sql.push_value(pragma_value)?; + sql.close_brace(); + let mut stmt = self.prepare(&sql)?; + let mut rows = stmt.query(NO_PARAMS)?; + while let Some(result_row) = rows.next() { + let row = result_row?; + f(&row)?; + } + Ok(()) + } + + /// Set a new value to `pragma_name`. + /// + /// Some pragmas will return the updated value which cannot be retrieved + /// with this method. + pub fn pragma_update( + &self, + schema_name: Option>, + pragma_name: &str, + pragma_value: &dyn ToSql, + ) -> Result<()> { + let mut sql = Sql::new(); + sql.push_pragma(schema_name, pragma_name)?; + // The argument is may be either in parentheses + // or it may be separated from the pragma name by an equal sign. + // The two syntaxes yield identical results. + sql.push_equal_sign(); + sql.push_value(pragma_value)?; + self.execute_batch(&sql) + } + + /// Set a new value to `pragma_name` and return the updated value. + /// + /// Only few pragmas automatically return the updated value. + pub fn pragma_update_and_check( + &self, + schema_name: Option>, + pragma_name: &str, + pragma_value: &dyn ToSql, + f: F, + ) -> Result + where + F: FnOnce(Row<'_, '_>) -> Result, + { + let mut sql = Sql::new(); + sql.push_pragma(schema_name, pragma_name)?; + // The argument is may be either in parentheses + // or it may be separated from the pragma name by an equal sign. + // The two syntaxes yield identical results. + sql.push_equal_sign(); + sql.push_value(pragma_value)?; + let mut stmt = self.prepare(&sql)?; + let mut rows = stmt.query(NO_PARAMS)?; + if let Some(result_row) = rows.next() { + let row = result_row?; + f(row) + } else { + Err(Error::QueryReturnedNoRows) + } + } +} + +fn is_identifier(s: &str) -> bool { + let chars = s.char_indices(); + for (i, ch) in chars { + if i == 0 { + if !is_identifier_start(ch) { + return false; + } + } else if !is_identifier_continue(ch) { + return false; + } + } + true +} + +fn is_identifier_start(c: char) -> bool { + (c >= 'A' && c <= 'Z') || c == '_' || (c >= 'a' && c <= 'z') || c > '\x7F' +} + +fn is_identifier_continue(c: char) -> bool { + c == '$' + || (c >= '0' && c <= '9') + || (c >= 'A' && c <= 'Z') + || c == '_' + || (c >= 'a' && c <= 'z') + || c > '\x7F' +} + +#[cfg(test)] +mod test { + use super::Sql; + use crate::pragma; + use crate::{Connection, DatabaseName}; + + #[test] + fn pragma_query_value() { + let db = Connection::open_in_memory().unwrap(); + let user_version: i32 = db + .pragma_query_value(None, "user_version", |row| row.get_checked(0)) + .unwrap(); + assert_eq!(0, user_version); + } + + #[test] + fn pragma_query_no_schema() { + let db = Connection::open_in_memory().unwrap(); + let mut user_version = -1; + db.pragma_query(None, "user_version", |row| { + user_version = row.get_checked(0)?; + Ok(()) + }) + .unwrap(); + assert_eq!(0, user_version); + } + + #[test] + fn pragma_query_with_schema() { + let db = Connection::open_in_memory().unwrap(); + let mut user_version = -1; + db.pragma_query(Some(DatabaseName::Main), "user_version", |row| { + user_version = row.get_checked(0)?; + Ok(()) + }) + .unwrap(); + assert_eq!(0, user_version); + } + + #[test] + fn pragma() { + let db = Connection::open_in_memory().unwrap(); + let mut columns = Vec::new(); + db.pragma(None, "table_info", &"sqlite_master", |row| { + let column: String = row.get_checked(1)?; + columns.push(column); + Ok(()) + }) + .unwrap(); + assert_eq!(5, columns.len()); + } + + #[test] + fn pragma_update() { + let db = Connection::open_in_memory().unwrap(); + db.pragma_update(None, "user_version", &1).unwrap(); + } + + #[test] + fn pragma_update_and_check() { + let db = Connection::open_in_memory().unwrap(); + let journal_mode: String = db + .pragma_update_and_check(None, "journal_mode", &"OFF", |row| row.get_checked(0)) + .unwrap(); + assert_eq!("off", &journal_mode); + } + + #[test] + fn is_identifier() { + assert!(pragma::is_identifier("full")); + assert!(pragma::is_identifier("r2d2")); + assert!(!pragma::is_identifier("sp ce")); + assert!(!pragma::is_identifier("semi;colon")); + } + + #[test] + fn double_quote() { + let mut sql = Sql::new(); + sql.push_schema_name(DatabaseName::Attached(r#"schema";--"#)); + assert_eq!(r#""schema"";--""#, sql.as_str()); + } + + #[test] + fn wrap_and_escape() { + let mut sql = Sql::new(); + sql.push_string_literal("value'; --"); + assert_eq!("'value''; --'", sql.as_str()); + } +} From 1b3a917ac8f86e3c48886d4543ca5e74ec13ae4b Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 9 Feb 2019 06:42:33 +0100 Subject: [PATCH 02/15] Fix nightly warning --- src/inner_connection.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/inner_connection.rs b/src/inner_connection.rs index 9842611..499dcd6 100644 --- a/src/inner_connection.rs +++ b/src/inner_connection.rs @@ -5,7 +5,7 @@ use std::os::raw::c_int; use std::path::Path; use std::ptr; use std::str; -use std::sync::atomic::{AtomicBool, Ordering, ATOMIC_BOOL_INIT}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, Once, ONCE_INIT}; use super::ffi; @@ -292,7 +292,7 @@ impl Drop for InnerConnection { #[cfg(not(feature = "bundled"))] static SQLITE_VERSION_CHECK: Once = ONCE_INIT; #[cfg(not(feature = "bundled"))] -pub static BYPASS_VERSION_CHECK: AtomicBool = ATOMIC_BOOL_INIT; +pub static BYPASS_VERSION_CHECK: AtomicBool = AtomicBool::new(false); #[cfg(not(feature = "bundled"))] fn ensure_valid_sqlite_version() { @@ -339,7 +339,7 @@ rusqlite was built against SQLite {} but the runtime SQLite version is {}. To fi } static SQLITE_INIT: Once = ONCE_INIT; -pub static BYPASS_SQLITE_INIT: AtomicBool = ATOMIC_BOOL_INIT; +pub static BYPASS_SQLITE_INIT: AtomicBool = AtomicBool::new(false); fn ensure_safe_sqlite_threading_mode() -> Result<()> { // Ensure SQLite was compiled in thredsafe mode. From d70286e98aca401be5332a7f481fb413658b4417 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 9 Feb 2019 07:16:05 +0100 Subject: [PATCH 03/15] Remove unwrap from examples --- README.md | 16 ++++++++-------- src/blob.rs | 27 ++++++++++++--------------- src/functions.rs | 10 +++++----- src/lib.rs | 19 ++++++++----------- 4 files changed, 33 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index f2d983e..46912b2 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ an interface similar to [rust-postgres](https://github.com/sfackler/rust-postgre ```rust use rusqlite::types::ToSql; -use rusqlite::{Connection, NO_PARAMS}; +use rusqlite::{Connection, Result, NO_PARAMS}; use time::Timespec; #[derive(Debug)] @@ -22,8 +22,8 @@ struct Person { data: Option>, } -fn main() { - let conn = Connection::open_in_memory().unwrap(); +fn main() -> Result<()> { + let conn = Connection::open_in_memory()?; conn.execute( "CREATE TABLE person ( @@ -33,7 +33,7 @@ fn main() { data BLOB )", NO_PARAMS, - ).unwrap(); + )?; let me = Person { id: 0, name: "Steven".to_string(), @@ -44,22 +44,22 @@ fn main() { "INSERT INTO person (name, time_created, data) VALUES (?1, ?2, ?3)", &[&me.name as &ToSql, &me.time_created, &me.data], - ).unwrap(); + )?; let mut stmt = conn - .prepare("SELECT id, name, time_created, data FROM person") - .unwrap(); + .prepare("SELECT id, name, time_created, data FROM person")?; let person_iter = stmt .query_map(NO_PARAMS, |row| Person { id: row.get(0), name: row.get(1), time_created: row.get(2), data: row.get(3), - }).unwrap(); + })?; for person in person_iter { println!("Found person {:?}", person.unwrap()); } + Ok(()) } ``` diff --git a/src/blob.rs b/src/blob.rs index 0f3d168..36d5b50 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -15,44 +15,41 @@ //! //! ```rust //! use rusqlite::blob::ZeroBlob; -//! use rusqlite::{Connection, DatabaseName, NO_PARAMS}; +//! use rusqlite::{Connection, DatabaseName, Result, NO_PARAMS}; //! use std::io::{Read, Seek, SeekFrom, Write}; //! -//! fn main() { -//! let db = Connection::open_in_memory().unwrap(); -//! db.execute_batch("CREATE TABLE test (content BLOB);") -//! .unwrap(); +//! fn main() -> Result<()> { +//! let db = Connection::open_in_memory()?; +//! db.execute_batch("CREATE TABLE test (content BLOB);")?; //! db.execute( //! "INSERT INTO test (content) VALUES (ZEROBLOB(10))", //! NO_PARAMS, -//! ) -//! .unwrap(); +//! )?; //! //! let rowid = db.last_insert_rowid(); //! let mut blob = db -//! .blob_open(DatabaseName::Main, "test", "content", rowid, false) -//! .unwrap(); +//! .blob_open(DatabaseName::Main, "test", "content", rowid, false)?; //! //! // Make sure to test that the number of bytes written matches what you expect; //! // if you try to write too much, the data will be truncated to the size of the //! // BLOB. -//! let bytes_written = blob.write(b"01234567").unwrap(); +//! let bytes_written = blob.write(b"01234567")?; //! assert_eq!(bytes_written, 8); //! //! // Same guidance - make sure you check the number of bytes read! -//! blob.seek(SeekFrom::Start(0)).unwrap(); +//! blob.seek(SeekFrom::Start(0))?; //! let mut buf = [0u8; 20]; -//! let bytes_read = blob.read(&mut buf[..]).unwrap(); +//! let bytes_read = blob.read(&mut buf[..])?; //! assert_eq!(bytes_read, 10); // note we read 10 bytes because the blob has size 10 //! -//! db.execute("INSERT INTO test (content) VALUES (?)", &[ZeroBlob(64)]) -//! .unwrap(); +//! db.execute("INSERT INTO test (content) VALUES (?)", &[ZeroBlob(64)])?; //! //! // given a new row ID, we can reopen the blob on that row //! let rowid = db.last_insert_rowid(); -//! blob.reopen(rowid).unwrap(); +//! blob.reopen(rowid)?; //! //! assert_eq!(blob.size(), 64); +//! Ok(()) //! } //! ``` use std::cmp::min; diff --git a/src/functions.rs b/src/functions.rs index ddacb84..c3ea25c 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -34,19 +34,19 @@ //! }) //! } //! -//! fn main() { -//! let db = Connection::open_in_memory().unwrap(); -//! add_regexp_function(&db).unwrap(); +//! fn main() -> Result<()> { +//! let db = Connection::open_in_memory()?; +//! add_regexp_function(&db)?; //! //! let is_match: bool = db //! .query_row( //! "SELECT regexp('[aeiou]*', 'aaaaeeeiii')", //! NO_PARAMS, //! |row| row.get(0), -//! ) -//! .unwrap(); +//! )?; //! //! assert!(is_match); +//! Ok(()) //! } //! ``` use std::error::Error as StdError; diff --git a/src/lib.rs b/src/lib.rs index a2dc60e..eddb794 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,7 +3,7 @@ //! //! ```rust //! use rusqlite::types::ToSql; -//! use rusqlite::{params, Connection}; +//! use rusqlite::{params, Connection, Result}; //! use time::Timespec; //! //! #[derive(Debug)] @@ -14,8 +14,8 @@ //! data: Option>, //! } //! -//! fn main() { -//! let conn = Connection::open_in_memory().unwrap(); +//! fn main() -> Result<()> { +//! let conn = Connection::open_in_memory()?; //! //! conn.execute( //! "CREATE TABLE person ( @@ -25,8 +25,7 @@ //! data BLOB //! )", //! params![], -//! ) -//! .unwrap(); +//! )?; //! let me = Person { //! id: 0, //! name: "Steven".to_string(), @@ -37,24 +36,22 @@ //! "INSERT INTO person (name, time_created, data) //! VALUES (?1, ?2, ?3)", //! params![me.name, me.time_created, me.data], -//! ) -//! .unwrap(); +//! )?; //! //! let mut stmt = conn -//! .prepare("SELECT id, name, time_created, data FROM person") -//! .unwrap(); +//! .prepare("SELECT id, name, time_created, data FROM person")?; //! let person_iter = stmt //! .query_map(params![], |row| Person { //! id: row.get(0), //! name: row.get(1), //! time_created: row.get(2), //! data: row.get(3), -//! }) -//! .unwrap(); +//! })?; //! //! for person in person_iter { //! println!("Found person {:?}", person.unwrap()); //! } +//! Ok(()) //! } //! ``` #![allow(unknown_lints)] From a8b9142d4750639c3d0c2b1ab22ed18b5ac60f78 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 9 Feb 2019 08:54:53 +0100 Subject: [PATCH 04/15] Fix Blob example --- src/blob.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/blob.rs b/src/blob.rs index 36d5b50..99d0013 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -15,10 +15,11 @@ //! //! ```rust //! use rusqlite::blob::ZeroBlob; -//! use rusqlite::{Connection, DatabaseName, Result, NO_PARAMS}; +//! use rusqlite::{Connection, DatabaseName, NO_PARAMS}; +//! use std::error::Error; //! use std::io::{Read, Seek, SeekFrom, Write}; //! -//! fn main() -> Result<()> { +//! fn main() -> Result<(), Box> { //! let db = Connection::open_in_memory()?; //! db.execute_batch("CREATE TABLE test (content BLOB);")?; //! db.execute( From 6ce5c9ddccb74453fce595fd8656f1d7d3dffc2e Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 14 Feb 2019 20:24:16 +0100 Subject: [PATCH 05/15] Suggest users to use PRAGMA function instead --- src/pragma.rs | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/src/pragma.rs b/src/pragma.rs index a9daff8..134582d 100644 --- a/src/pragma.rs +++ b/src/pragma.rs @@ -161,6 +161,9 @@ impl Connection { /// /// Some pragmas will return multiple rows/values which cannot be retrieved /// with this method. + /// + /// Prefer [PRAGMA function](https://sqlite.org/pragma.html#pragfunc) introduced in SQLite 3.20: + /// `SELECT user_version FROM pragma_user_version;` pub fn pragma_query_value( &self, schema_name: Option>, @@ -183,6 +186,9 @@ impl Connection { } /// Query the current rows/values of `pragma_name`. + /// + /// Prefer [PRAGMA function](https://sqlite.org/pragma.html#pragfunc) introduced in SQLite 3.20: + /// `SELECT * FROM pragma_collation_list;` pub fn pragma_query( &self, schema_name: Option>, @@ -207,8 +213,11 @@ impl Connection { /// `pragma_value`. /// /// This method can be used with query-only pragmas which need an argument - /// (e.g. `table_info('one_tbl')`) or pragmas which return the updated value - /// (e.g. `journal_mode`). + /// (e.g. `table_info('one_tbl')`) or pragmas which returns value(s) + /// (e.g. `integrity_check`). + /// + /// Prefer [PRAGMA function](https://sqlite.org/pragma.html#pragfunc) introduced in SQLite 3.20: + /// `SELECT * FROM pragma_table_info(?);` pub fn pragma( &self, schema_name: Option>, @@ -318,7 +327,7 @@ fn is_identifier_continue(c: char) -> bool { mod test { use super::Sql; use crate::pragma; - use crate::{Connection, DatabaseName}; + use crate::{Connection, DatabaseName, NO_PARAMS}; #[test] fn pragma_query_value() { @@ -329,6 +338,20 @@ mod test { assert_eq!(0, user_version); } + #[test] + #[cfg(feature = "bundled")] + fn pragma_func_query_value() { + let db = Connection::open_in_memory().unwrap(); + let user_version: i32 = db + .query_row( + "SELECT user_version FROM pragma_user_version", + NO_PARAMS, + |row| row.get(0), + ) + .unwrap(); + assert_eq!(0, user_version); + } + #[test] fn pragma_query_no_schema() { let db = Connection::open_in_memory().unwrap(); @@ -366,6 +389,22 @@ mod test { assert_eq!(5, columns.len()); } + #[test] + #[cfg(feature = "bundled")] + fn pragma_func() { + let db = Connection::open_in_memory().unwrap(); + let mut table_info = db.prepare("SELECT * FROM pragma_table_info(?)").unwrap(); + let mut columns = Vec::new(); + let mut rows = table_info.query(&["sqlite_master"]).unwrap(); + + while let Some(row) = rows.next() { + let row = row.unwrap(); + let column: String = row.get(1); + columns.push(column); + } + assert_eq!(5, columns.len()); + } + #[test] fn pragma_update() { let db = Connection::open_in_memory().unwrap(); From 364e885b8904d115737fe71e8bdfd1576afe8326 Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 14 Feb 2019 20:53:00 +0100 Subject: [PATCH 06/15] Fix warning --- src/pragma.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pragma.rs b/src/pragma.rs index 134582d..d7f2144 100644 --- a/src/pragma.rs +++ b/src/pragma.rs @@ -327,7 +327,7 @@ fn is_identifier_continue(c: char) -> bool { mod test { use super::Sql; use crate::pragma; - use crate::{Connection, DatabaseName, NO_PARAMS}; + use crate::{Connection, DatabaseName}; #[test] fn pragma_query_value() { @@ -341,6 +341,8 @@ mod test { #[test] #[cfg(feature = "bundled")] fn pragma_func_query_value() { + use crate::NO_PARAMS; + let db = Connection::open_in_memory().unwrap(); let user_version: i32 = db .query_row( From 1d1ee97ab971d7b78658f6dbd3ef1a5f869b5afd Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 14 Feb 2019 20:53:26 +0100 Subject: [PATCH 07/15] Try to fix AppVeyor build --- appveyor.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index db0bfad..acf29ba 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -16,10 +16,10 @@ install: - rustc -V - cargo -V # download SQLite dll (useful only when the `bundled` feature is not set) - - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-dll-win64-x64-3250000.zip -FileName sqlite-dll-win64-x64.zip + - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-dll-win64-x64-3260000.zip -FileName sqlite-dll-win64-x64.zip - if not defined VCPKG_DEFAULT_TRIPLET 7z e sqlite-dll-win64-x64.zip -y > nul # download SQLite headers (useful only when the `bundled` feature is not set) - - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-amalgamation-3250000.zip -FileName sqlite-amalgamation.zip + - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-amalgamation-3260000.zip -FileName sqlite-amalgamation.zip - if not defined VCPKG_DEFAULT_TRIPLET 7z e sqlite-amalgamation.zip -y > nul # specify where the SQLite dll has been downloaded (useful only when the `bundled` feature is not set) - if not defined VCPKG_DEFAULT_TRIPLET SET SQLITE3_LIB_DIR=%APPVEYOR_BUILD_FOLDER% @@ -33,10 +33,10 @@ build: false test_script: - cargo test --lib --verbose - cargo test --lib --verbose --features bundled - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab" - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab buildtime_bindgen" - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab bundled" - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab bundled buildtime_bindgen" + - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3" + - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3 buildtime_bindgen" + - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3 bundled" + - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3 bundled buildtime_bindgen" cache: - C:\Users\appveyor\.cargo From b6470f371fc1d5815a755c4922bb2b59a9c71938 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 16 Feb 2019 10:07:55 +0100 Subject: [PATCH 08/15] Try to fix AppVeyor build --- appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index acf29ba..e4775b0 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -33,8 +33,8 @@ build: false test_script: - cargo test --lib --verbose - cargo test --lib --verbose --features bundled - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3" - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3 buildtime_bindgen" + - cargo test --lib --features "backup blob chrono functions hooks limits load_extension serde_json trace" + - cargo test --lib --features "backup blob chrono functions hooks limits load_extension serde_json trace buildtime_bindgen" - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3 bundled" - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab_v3 bundled buildtime_bindgen" From 37027a6087235115366439d22624d0124216d361 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 16 Feb 2019 11:06:21 +0100 Subject: [PATCH 09/15] Try to fix AppVeyor build --- appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index e4775b0..5e4761f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -16,10 +16,10 @@ install: - rustc -V - cargo -V # download SQLite dll (useful only when the `bundled` feature is not set) - - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-dll-win64-x64-3260000.zip -FileName sqlite-dll-win64-x64.zip + - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-dll-win64-x64-3250200.zip -FileName sqlite-dll-win64-x64.zip - if not defined VCPKG_DEFAULT_TRIPLET 7z e sqlite-dll-win64-x64.zip -y > nul # download SQLite headers (useful only when the `bundled` feature is not set) - - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-amalgamation-3260000.zip -FileName sqlite-amalgamation.zip + - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-amalgamation-3250200.zip -FileName sqlite-amalgamation.zip - if not defined VCPKG_DEFAULT_TRIPLET 7z e sqlite-amalgamation.zip -y > nul # specify where the SQLite dll has been downloaded (useful only when the `bundled` feature is not set) - if not defined VCPKG_DEFAULT_TRIPLET SET SQLITE3_LIB_DIR=%APPVEYOR_BUILD_FOLDER% From c1f12c7380e639b9a5241278243104eb739e24d9 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 16 Feb 2019 17:24:56 +0100 Subject: [PATCH 10/15] Revert change related to #460 --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index eddb794..ee2f513 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -687,7 +687,7 @@ impl Connection { /// Return the number of rows modified, inserted or deleted by the most /// recently completed INSERT, UPDATE or DELETE statement on the database /// connection. - pub fn changes(&self) -> usize { + fn changes(&self) -> usize { self.db.borrow_mut().changes() } From aa2179d306d35cb9c0bdd4ab09ea040ab3d1d9be Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 16 Feb 2019 17:51:21 +0100 Subject: [PATCH 11/15] Try to fix AppVeyor build --- appveyor.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index db0bfad..469e493 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -16,10 +16,10 @@ install: - rustc -V - cargo -V # download SQLite dll (useful only when the `bundled` feature is not set) - - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-dll-win64-x64-3250000.zip -FileName sqlite-dll-win64-x64.zip + - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-dll-win64-x64-3250200.zip -FileName sqlite-dll-win64-x64.zip - if not defined VCPKG_DEFAULT_TRIPLET 7z e sqlite-dll-win64-x64.zip -y > nul # download SQLite headers (useful only when the `bundled` feature is not set) - - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-amalgamation-3250000.zip -FileName sqlite-amalgamation.zip + - appveyor-retry appveyor DownloadFile https://sqlite.org/2018/sqlite-amalgamation-3250200.zip -FileName sqlite-amalgamation.zip - if not defined VCPKG_DEFAULT_TRIPLET 7z e sqlite-amalgamation.zip -y > nul # specify where the SQLite dll has been downloaded (useful only when the `bundled` feature is not set) - if not defined VCPKG_DEFAULT_TRIPLET SET SQLITE3_LIB_DIR=%APPVEYOR_BUILD_FOLDER% @@ -33,8 +33,8 @@ build: false test_script: - cargo test --lib --verbose - cargo test --lib --verbose --features bundled - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab" - - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab buildtime_bindgen" + - cargo test --lib --features "backup blob chrono functions hooks limits load_extension serde_json trace" + - cargo test --lib --features "backup blob chrono functions hooks limits load_extension serde_json trace buildtime_bindgen" - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab bundled" - cargo test --lib --features "backup blob chrono csvtab functions hooks limits load_extension serde_json trace vtab bundled buildtime_bindgen" From fcaf5b9dd8711fa1a60f55d9ed2c907ae39a178e Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 21 Feb 2019 18:55:51 +0100 Subject: [PATCH 12/15] Fix typos --- src/pragma.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pragma.rs b/src/pragma.rs index d7f2144..50302be 100644 --- a/src/pragma.rs +++ b/src/pragma.rs @@ -230,7 +230,7 @@ impl Connection { { let mut sql = Sql::new(); sql.push_pragma(schema_name, pragma_name)?; - // The argument is may be either in parentheses + // The argument may be either in parentheses // or it may be separated from the pragma name by an equal sign. // The two syntaxes yield identical results. sql.open_brace(); @@ -257,7 +257,7 @@ impl Connection { ) -> Result<()> { let mut sql = Sql::new(); sql.push_pragma(schema_name, pragma_name)?; - // The argument is may be either in parentheses + // The argument may be either in parentheses // or it may be separated from the pragma name by an equal sign. // The two syntaxes yield identical results. sql.push_equal_sign(); @@ -280,7 +280,7 @@ impl Connection { { let mut sql = Sql::new(); sql.push_pragma(schema_name, pragma_name)?; - // The argument is may be either in parentheses + // The argument may be either in parentheses // or it may be separated from the pragma name by an equal sign. // The two syntaxes yield identical results. sql.push_equal_sign(); From 6d9ae896b5e3553a31c3c0aa7648e7851f1d37ff Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 21 Feb 2019 20:48:09 +0100 Subject: [PATCH 13/15] Do not panic by default Replace `Row::get` by `Row::get_checked`, And rename original `Row::get` to `Row::get_unwrap`. `Stmt::query_map`, `Stmt::query_map_named`, `Stmt::query_row`, `Conn::query_row` and `Conn::query_row_named` callback parameter must return a `Result`. --- README.md | 12 ++--- src/blob.rs | 3 +- src/busy.rs | 8 +-- src/cache.rs | 2 +- src/config.rs | 60 ++++++++++++++++------- src/functions.rs | 13 +++-- src/lib.rs | 63 +++++++++++------------- src/row.rs | 22 ++++----- src/statement.rs | 22 ++++----- src/trace.rs | 8 +-- src/types/from_sql.rs | 3 +- src/types/mod.rs | 111 +++++++++++++++++++----------------------- src/types/to_sql.rs | 2 +- src/vtab/csvtab.rs | 2 +- 14 files changed, 169 insertions(+), 162 deletions(-) diff --git a/README.md b/README.md index 46912b2..268f23f 100644 --- a/README.md +++ b/README.md @@ -49,12 +49,12 @@ fn main() -> Result<()> { let mut stmt = conn .prepare("SELECT id, name, time_created, data FROM person")?; let person_iter = stmt - .query_map(NO_PARAMS, |row| Person { - id: row.get(0), - name: row.get(1), - time_created: row.get(2), - data: row.get(3), - })?; + .query_map(NO_PARAMS, |row| Ok(Person { + id: row.get(0)?, + name: row.get(1)?, + time_created: row.get(2)?, + data: row.get(3)?, + }))?; for person in person_iter { println!("Found person {:?}", person.unwrap()); diff --git a/src/blob.rs b/src/blob.rs index 99d0013..372fc4c 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -28,8 +28,7 @@ //! )?; //! //! let rowid = db.last_insert_rowid(); -//! let mut blob = db -//! .blob_open(DatabaseName::Main, "test", "content", rowid, false)?; +//! let mut blob = db.blob_open(DatabaseName::Main, "test", "content", rowid, false)?; //! //! // Make sure to test that the number of bytes written matches what you expect; //! // if you try to write too much, the data will be truncated to the size of the diff --git a/src/busy.rs b/src/busy.rs index e0ee835..f801e3f 100644 --- a/src/busy.rs +++ b/src/busy.rs @@ -82,7 +82,7 @@ mod test { use std::time::Duration; use tempdir; - use crate::{Connection, Error, ErrorCode, TransactionBehavior, NO_PARAMS}; + use crate::{Connection, Error, ErrorCode, Result, TransactionBehavior, NO_PARAMS}; #[test] fn test_default_busy() { @@ -94,7 +94,7 @@ mod test { .transaction_with_behavior(TransactionBehavior::Exclusive) .unwrap(); let db2 = Connection::open(&path).unwrap(); - let r = db2.query_row("PRAGMA schema_version", NO_PARAMS, |_| unreachable!()); + let r: Result<()> = db2.query_row("PRAGMA schema_version", NO_PARAMS, |_| unreachable!()); match r.unwrap_err() { Error::SqliteFailure(err, _) => { assert_eq!(err.code, ErrorCode::DatabaseBusy); @@ -127,7 +127,7 @@ mod test { assert_eq!(tx.recv().unwrap(), 1); let _ = db2 .query_row("PRAGMA schema_version", NO_PARAMS, |row| { - row.get_checked::<_, i32>(0) + row.get::<_, i32>(0) }) .expect("unexpected error"); @@ -166,7 +166,7 @@ mod test { assert_eq!(tx.recv().unwrap(), 1); let _ = db2 .query_row("PRAGMA schema_version", NO_PARAMS, |row| { - row.get_checked::<_, i32>(0) + row.get::<_, i32>(0) }) .expect("unexpected error"); assert_eq!(CALLED.load(Ordering::Relaxed), true); diff --git a/src/cache.rs b/src/cache.rs index 2c44641..a93410f 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -298,7 +298,7 @@ mod test { let mut stmt = db.prepare_cached(sql).unwrap(); assert_eq!( (1i32, 2i32), - stmt.query_map(NO_PARAMS, |r| (r.get(0), r.get(1))) + stmt.query_map(NO_PARAMS, |r| Ok((r.get(0)?, r.get(1)?))) .unwrap() .next() .unwrap() diff --git a/src/config.rs b/src/config.rs index 36337f0..a10faac 100644 --- a/src/config.rs +++ b/src/config.rs @@ -25,12 +25,18 @@ pub enum DbConfig { impl Connection { /// Returns the current value of a `config`. /// - /// - SQLITE_DBCONFIG_ENABLE_FKEY: return `false` or `true` to indicate whether FK enforcement is off or on - /// - SQLITE_DBCONFIG_ENABLE_TRIGGER: return `false` or `true` to indicate whether triggers are disabled or enabled - /// - SQLITE_DBCONFIG_ENABLE_FTS3_TOKENIZER: return `false` or `true` to indicate whether fts3_tokenizer are disabled or enabled - /// - SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE: return `false` to indicate checkpoints-on-close are not disabled or `true` if they are - /// - SQLITE_DBCONFIG_ENABLE_QPSG: return `false` or `true` to indicate whether the QPSG is disabled or enabled - /// - SQLITE_DBCONFIG_TRIGGER_EQP: return `false` to indicate output-for-trigger are not disabled or `true` if it is + /// - SQLITE_DBCONFIG_ENABLE_FKEY: return `false` or `true` to indicate + /// whether FK enforcement is off or on + /// - SQLITE_DBCONFIG_ENABLE_TRIGGER: return `false` or `true` to indicate + /// whether triggers are disabled or enabled + /// - SQLITE_DBCONFIG_ENABLE_FTS3_TOKENIZER: return `false` or `true` to + /// indicate whether fts3_tokenizer are disabled or enabled + /// - SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE: return `false` to indicate + /// checkpoints-on-close are not disabled or `true` if they are + /// - SQLITE_DBCONFIG_ENABLE_QPSG: return `false` or `true` to indicate + /// whether the QPSG is disabled or enabled + /// - SQLITE_DBCONFIG_TRIGGER_EQP: return `false` to indicate + /// output-for-trigger are not disabled or `true` if it is pub fn db_config(&self, config: DbConfig) -> Result { let c = self.db.borrow(); unsafe { @@ -47,12 +53,18 @@ impl Connection { /// Make configuration changes to a database connection /// - /// - SQLITE_DBCONFIG_ENABLE_FKEY: `false` to disable FK enforcement, `true` to enable FK enforcement - /// - SQLITE_DBCONFIG_ENABLE_TRIGGER: `false` to disable triggers, `true` to enable triggers - /// - SQLITE_DBCONFIG_ENABLE_FTS3_TOKENIZER: `false` to disable fts3_tokenizer(), `true` to enable fts3_tokenizer() - /// - SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE: `false` (the default) to enable checkpoints-on-close, `true` to disable them - /// - SQLITE_DBCONFIG_ENABLE_QPSG: `false` to disable the QPSG, `true` to enable QPSG - /// - SQLITE_DBCONFIG_TRIGGER_EQP: `false` to disable output for trigger programs, `true` to enable it + /// - SQLITE_DBCONFIG_ENABLE_FKEY: `false` to disable FK enforcement, `true` + /// to enable FK enforcement + /// - SQLITE_DBCONFIG_ENABLE_TRIGGER: `false` to disable triggers, `true` to + /// enable triggers + /// - SQLITE_DBCONFIG_ENABLE_FTS3_TOKENIZER: `false` to disable + /// fts3_tokenizer(), `true` to enable fts3_tokenizer() + /// - SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE: `false` (the default) to enable + /// checkpoints-on-close, `true` to disable them + /// - SQLITE_DBCONFIG_ENABLE_QPSG: `false` to disable the QPSG, `true` to + /// enable QPSG + /// - SQLITE_DBCONFIG_TRIGGER_EQP: `false` to disable output for trigger + /// programs, `true` to enable it pub fn set_db_config(&self, config: DbConfig, new_val: bool) -> Result { let c = self.db.borrow_mut(); unsafe { @@ -78,11 +90,25 @@ mod test { let db = Connection::open_in_memory().unwrap(); let opposite = !db.db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_FKEY).unwrap(); - assert_eq!(db.set_db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_FKEY, opposite), Ok(opposite)); - assert_eq!(db.db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_FKEY), Ok(opposite)); + assert_eq!( + db.set_db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_FKEY, opposite), + Ok(opposite) + ); + assert_eq!( + db.db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_FKEY), + Ok(opposite) + ); - let opposite = !db.db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER).unwrap(); - assert_eq!(db.set_db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER, opposite), Ok(opposite)); - assert_eq!(db.db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER), Ok(opposite)); + let opposite = !db + .db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER) + .unwrap(); + assert_eq!( + db.set_db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER, opposite), + Ok(opposite) + ); + assert_eq!( + db.db_config(DbConfig::SQLITE_DBCONFIG_ENABLE_TRIGGER), + Ok(opposite) + ); } } diff --git a/src/functions.rs b/src/functions.rs index c3ea25c..a4773a0 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -38,12 +38,11 @@ //! let db = Connection::open_in_memory()?; //! add_regexp_function(&db)?; //! -//! let is_match: bool = db -//! .query_row( -//! "SELECT regexp('[aeiou]*', 'aaaaeeeiii')", -//! NO_PARAMS, -//! |row| row.get(0), -//! )?; +//! let is_match: bool = db.query_row( +//! "SELECT regexp('[aeiou]*', 'aaaaeeeiii')", +//! NO_PARAMS, +//! |row| row.get(0), +//! )?; //! //! assert!(is_match); //! Ok(()) @@ -771,7 +770,7 @@ mod test { let dual_sum = "SELECT my_sum(i), my_sum(j) FROM (SELECT 2 AS i, 1 AS j UNION ALL SELECT \ 2, 1)"; let result: (i64, i64) = db - .query_row(dual_sum, NO_PARAMS, |r| (r.get(0), r.get(1))) + .query_row(dual_sum, NO_PARAMS, |r| Ok((r.get(0)?, r.get(1)?))) .unwrap(); assert_eq!((4, 2), result); } diff --git a/src/lib.rs b/src/lib.rs index ee2f513..a06a25f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,15 +38,15 @@ //! params![me.name, me.time_created, me.data], //! )?; //! -//! let mut stmt = conn -//! .prepare("SELECT id, name, time_created, data FROM person")?; -//! let person_iter = stmt -//! .query_map(params![], |row| Person { -//! id: row.get(0), -//! name: row.get(1), -//! time_created: row.get(2), -//! data: row.get(3), -//! })?; +//! let mut stmt = conn.prepare("SELECT id, name, time_created, data FROM person")?; +//! let person_iter = stmt.query_map(params![], |row| { +//! Ok(Person { +//! id: row.get(0)?, +//! name: row.get(1)?, +//! time_created: row.get(2)?, +//! data: row.get(3)?, +//! }) +//! })?; //! //! for person in person_iter { //! println!("Found person {:?}", person.unwrap()); @@ -473,7 +473,7 @@ impl Connection { where P: IntoIterator, P::Item: ToSql, - F: FnOnce(&Row<'_, '_>) -> T, + F: FnOnce(&Row<'_, '_>) -> Result, { let mut stmt = self.prepare(sql)?; stmt.query_row(params, f) @@ -495,12 +495,12 @@ impl Connection { /// or if the underlying SQLite call fails. pub fn query_row_named(&self, sql: &str, params: &[(&str, &dyn ToSql)], f: F) -> Result where - F: FnOnce(&Row<'_, '_>) -> T, + F: FnOnce(&Row<'_, '_>) -> Result, { let mut stmt = self.prepare(sql)?; let mut rows = stmt.query_named(params)?; - rows.get_expected_row().map(|r| f(&r)) + rows.get_expected_row().and_then(|r| f(&r)) } /// Convenience method to execute a query that is expected to return a @@ -516,7 +516,7 @@ impl Connection { /// conn.query_row_and_then( /// "SELECT value FROM preferences WHERE name='locale'", /// NO_PARAMS, - /// |row| row.get_checked(0), + /// |row| row.get(0), /// ) /// } /// ``` @@ -858,9 +858,9 @@ mod test { let tx2 = db2.transaction().unwrap(); // SELECT first makes sqlite lock with a shared lock - tx1.query_row("SELECT x FROM foo LIMIT 1", NO_PARAMS, |_| ()) + tx1.query_row("SELECT x FROM foo LIMIT 1", NO_PARAMS, |_| Ok(())) .unwrap(); - tx2.query_row("SELECT x FROM foo LIMIT 1", NO_PARAMS, |_| ()) + tx2.query_row("SELECT x FROM foo LIMIT 1", NO_PARAMS, |_| Ok(())) .unwrap(); tx1.execute("INSERT INTO foo VALUES(?1)", &[1]).unwrap(); @@ -1061,7 +1061,7 @@ mod test { let mut v = Vec::::new(); while let Some(row) = rows.next() { - v.push(row.unwrap().get(0)); + v.push(row.unwrap().get(0).unwrap()); } assert_eq!(v, [3i32, 2, 1]); @@ -1072,7 +1072,7 @@ mod test { let mut v = Vec::::new(); while let Some(row) = rows.next() { - v.push(row.unwrap().get(0)); + v.push(row.unwrap().get(0).unwrap()); } assert_eq!(v, [2i32, 1]); @@ -1125,7 +1125,7 @@ mod test { err => panic!("Unexpected error {}", err), } - let bad_query_result = db.query_row("NOT A PROPER QUERY; test123", NO_PARAMS, |_| ()); + let bad_query_result = db.query_row("NOT A PROPER QUERY; test123", NO_PARAMS, |_| Ok(())); assert!(bad_query_result.is_err()); } @@ -1400,7 +1400,7 @@ mod test { let mut query = db.prepare("SELECT x, y FROM foo ORDER BY x DESC").unwrap(); let results: Result> = query - .query_and_then(NO_PARAMS, |row| row.get_checked(1)) + .query_and_then(NO_PARAMS, |row| row.get(1)) .unwrap() .collect(); @@ -1421,7 +1421,7 @@ mod test { let mut query = db.prepare("SELECT x, y FROM foo ORDER BY x DESC").unwrap(); let bad_type: Result> = query - .query_and_then(NO_PARAMS, |row| row.get_checked(1)) + .query_and_then(NO_PARAMS, |row| row.get(1)) .unwrap() .collect(); @@ -1431,7 +1431,7 @@ mod test { } let bad_idx: Result> = query - .query_and_then(NO_PARAMS, |row| row.get_checked(3)) + .query_and_then(NO_PARAMS, |row| row.get(3)) .unwrap() .collect(); @@ -1455,9 +1455,7 @@ mod test { let mut query = db.prepare("SELECT x, y FROM foo ORDER BY x DESC").unwrap(); let results: CustomResult> = query - .query_and_then(NO_PARAMS, |row| { - row.get_checked(1).map_err(CustomError::Sqlite) - }) + .query_and_then(NO_PARAMS, |row| row.get(1).map_err(CustomError::Sqlite)) .unwrap() .collect(); @@ -1478,9 +1476,7 @@ mod test { let mut query = db.prepare("SELECT x, y FROM foo ORDER BY x DESC").unwrap(); let bad_type: CustomResult> = query - .query_and_then(NO_PARAMS, |row| { - row.get_checked(1).map_err(CustomError::Sqlite) - }) + .query_and_then(NO_PARAMS, |row| row.get(1).map_err(CustomError::Sqlite)) .unwrap() .collect(); @@ -1490,9 +1486,7 @@ mod test { } let bad_idx: CustomResult> = query - .query_and_then(NO_PARAMS, |row| { - row.get_checked(3).map_err(CustomError::Sqlite) - }) + .query_and_then(NO_PARAMS, |row| row.get(3).map_err(CustomError::Sqlite)) .unwrap() .collect(); @@ -1523,7 +1517,7 @@ mod test { let query = "SELECT x, y FROM foo ORDER BY x DESC"; let results: CustomResult = db.query_row_and_then(query, NO_PARAMS, |row| { - row.get_checked(1).map_err(CustomError::Sqlite) + row.get(1).map_err(CustomError::Sqlite) }); assert_eq!(results.unwrap(), "hello"); @@ -1540,7 +1534,7 @@ mod test { let query = "SELECT x, y FROM foo ORDER BY x DESC"; let bad_type: CustomResult = db.query_row_and_then(query, NO_PARAMS, |row| { - row.get_checked(1).map_err(CustomError::Sqlite) + row.get(1).map_err(CustomError::Sqlite) }); match bad_type.unwrap_err() { @@ -1549,7 +1543,7 @@ mod test { } let bad_idx: CustomResult = db.query_row_and_then(query, NO_PARAMS, |row| { - row.get_checked(3).map_err(CustomError::Sqlite) + row.get(3).map_err(CustomError::Sqlite) }); match bad_idx.unwrap_err() { @@ -1576,7 +1570,8 @@ mod test { db.execute_batch(sql).unwrap(); db.query_row("SELECT * FROM foo", params![], |r| { - assert_eq!(2, r.column_count()) + assert_eq!(2, r.column_count()); + Ok(()) }) .unwrap(); } diff --git a/src/row.rs b/src/row.rs index e0381a1..27bc69e 100644 --- a/src/row.rs +++ b/src/row.rs @@ -73,7 +73,7 @@ pub struct MappedRows<'stmt, F> { impl<'stmt, T, F> MappedRows<'stmt, F> where - F: FnMut(&Row<'_, '_>) -> T, + F: FnMut(&Row<'_, '_>) -> Result, { pub(crate) fn new(rows: Rows<'stmt>, f: F) -> MappedRows<'stmt, F> { MappedRows { rows, map: f } @@ -82,7 +82,7 @@ where impl Iterator for MappedRows<'_, F> where - F: FnMut(&Row<'_, '_>) -> T, + F: FnMut(&Row<'_, '_>) -> Result, { type Item = Result; @@ -90,7 +90,7 @@ where let map = &mut self.map; self.rows .next() - .map(|row_result| row_result.map(|row| (map)(&row))) + .map(|row_result| row_result.and_then(|row| (map)(&row))) } } @@ -136,16 +136,16 @@ impl<'a, 'stmt> Row<'a, 'stmt> { /// /// ## Failure /// - /// Panics if calling `row.get_checked(idx)` would return an error, + /// Panics if calling `row.get(idx)` would return an error, /// including: /// - /// * If the underlying SQLite column type is not a valid type as a - /// source for `T` + /// * If the underlying SQLite column type is not a valid type as a source + /// for `T` /// * If the underlying SQLite integral value is outside the range /// representable by `T` /// * If `idx` is outside the range of columns in the returned query - pub fn get(&self, idx: I) -> T { - self.get_checked(idx).unwrap() + pub fn get_unwrap(&self, idx: I) -> T { + self.get(idx).unwrap() } /// Get the value of a particular column of the result row. @@ -164,7 +164,7 @@ impl<'a, 'stmt> Row<'a, 'stmt> { /// If the result type is i128 (which requires the `i128_blob` feature to be /// enabled), and the underlying SQLite column is a blob whose size is not /// 16 bytes, `Error::InvalidColumnType` will also be returned. - pub fn get_checked(&self, idx: I) -> Result { + pub fn get(&self, idx: I) -> Result { let idx = idx.idx(self.stmt)?; let value = self.stmt.value_ref(idx); FromSql::column_result(value).map_err(|err| match err { @@ -184,7 +184,7 @@ impl<'a, 'stmt> Row<'a, 'stmt> { /// This `ValueRef` is valid only as long as this Row, which is enforced by /// it's lifetime. This means that while this method is completely safe, /// it can be somewhat difficult to use, and most callers will be better - /// served by `get` or `get_checked`. + /// served by `get` or `get`. /// /// ## Failure /// @@ -208,7 +208,7 @@ impl<'a, 'stmt> Row<'a, 'stmt> { /// This `ValueRef` is valid only as long as this Row, which is enforced by /// it's lifetime. This means that while this method is completely safe, /// it can be difficult to use, and most callers will be better served by - /// `get` or `get_checked`. + /// `get` or `get`. /// /// ## Failure /// diff --git a/src/statement.rs b/src/statement.rs index a32aa28..aa07ea7 100644 --- a/src/statement.rs +++ b/src/statement.rs @@ -176,7 +176,7 @@ impl Statement<'_> { /// let mut names = Vec::new(); /// while let Some(result_row) = rows.next() { /// let row = result_row?; - /// names.push(row.get(0)); + /// names.push(row.get(0)?); /// } /// /// Ok(names) @@ -267,7 +267,7 @@ impl Statement<'_> { where P: IntoIterator, P::Item: ToSql, - F: FnMut(&Row<'_, '_>) -> T, + F: FnMut(&Row<'_, '_>) -> Result, { let rows = self.query(params)?; Ok(MappedRows::new(rows, f)) @@ -306,7 +306,7 @@ impl Statement<'_> { f: F, ) -> Result> where - F: FnMut(&Row<'_, '_>) -> T, + F: FnMut(&Row<'_, '_>) -> Result, { let rows = self.query_named(params)?; Ok(MappedRows::new(rows, f)) @@ -354,7 +354,7 @@ impl Statement<'_> { /// fn get_names(conn: &Connection) -> Result> { /// let mut stmt = conn.prepare("SELECT name FROM people WHERE id = :id")?; /// let rows = - /// stmt.query_and_then_named(&[(":id", &"one")], |row| name_to_person(row.get(0)))?; + /// stmt.query_and_then_named(&[(":id", &"one")], |row| name_to_person(row.get(0)?))?; /// /// let mut persons = Vec::new(); /// for person_result in rows { @@ -410,11 +410,11 @@ impl Statement<'_> { where P: IntoIterator, P::Item: ToSql, - F: FnOnce(&Row<'_, '_>) -> T, + F: FnOnce(&Row<'_, '_>) -> Result, { let mut rows = self.query(params)?; - rows.get_expected_row().map(|r| f(&r)) + rows.get_expected_row().and_then(|r| f(&r)) } /// Consumes the statement. @@ -798,8 +798,8 @@ mod test { .unwrap(); let mut rows = stmt.query_named(&[(":name", &"one")]).unwrap(); - let id: i32 = rows.next().unwrap().unwrap().get(0); - assert_eq!(1, id); + let id: Result = rows.next().unwrap().and_then(|row| row.get(0)); + assert_eq!(Ok(1), id); } #[test] @@ -816,8 +816,8 @@ mod test { .unwrap(); let mut rows = stmt .query_map_named(&[(":name", &"one")], |row| { - let id: i32 = row.get(0); - 2 * id + let id: Result = row.get(0); + id.map(|i| 2 * i) }) .unwrap(); @@ -840,7 +840,7 @@ mod test { .unwrap(); let mut rows = stmt .query_and_then_named(&[(":name", &"one")], |row| { - let id: i32 = row.get(0); + let id: i32 = row.get(0)?; if id == 1 { Ok(id) } else { diff --git a/src/trace.rs b/src/trace.rs index ddf537f..8af3f27 100644 --- a/src/trace.rs +++ b/src/trace.rs @@ -140,13 +140,13 @@ mod test { let mut db = Connection::open_in_memory().unwrap(); db.trace(Some(tracer)); { - let _ = db.query_row("SELECT ?", &[1i32], |_| {}); - let _ = db.query_row("SELECT ?", &["hello"], |_| {}); + let _ = db.query_row("SELECT ?", &[1i32], |_| Ok(())); + let _ = db.query_row("SELECT ?", &["hello"], |_| Ok(())); } db.trace(None); { - let _ = db.query_row("SELECT ?", &[2i32], |_| {}); - let _ = db.query_row("SELECT ?", &["goodbye"], |_| {}); + let _ = db.query_row("SELECT ?", &[2i32], |_| Ok(())); + let _ = db.query_row("SELECT ?", &["goodbye"], |_| Ok(())); } let traced_stmts = TRACED_STMTS.lock().unwrap(); diff --git a/src/types/from_sql.rs b/src/types/from_sql.rs index c917b7a..1264c25 100644 --- a/src/types/from_sql.rs +++ b/src/types/from_sql.rs @@ -210,8 +210,7 @@ mod test { { for n in out_of_range { let err = db - .query_row("SELECT ?", &[n], |r| r.get_checked::<_, T>(0)) - .unwrap() + .query_row("SELECT ?", &[n], |r| r.get::<_, T>(0)) .unwrap_err(); match err { Error::IntegralValueOutOfRange(_, value) => assert_eq!(*n, value), diff --git a/src/types/mod.rs b/src/types/mod.rs index 508b273..48aa60a 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -207,16 +207,16 @@ mod test { { let row1 = rows.next().unwrap().unwrap(); - let s1: Option = row1.get(0); - let b1: Option> = row1.get(1); + let s1: Option = row1.get_unwrap(0); + let b1: Option> = row1.get_unwrap(1); assert_eq!(s.unwrap(), s1.unwrap()); assert!(b1.is_none()); } { let row2 = rows.next().unwrap().unwrap(); - let s2: Option = row2.get(0); - let b2: Option> = row2.get(1); + let s2: Option = row2.get_unwrap(0); + let b2: Option> = row2.get_unwrap(1); assert!(s2.is_none()); assert_eq!(b, b2); } @@ -246,102 +246,94 @@ mod test { let row = rows.next().unwrap().unwrap(); // check the correct types come back as expected - assert_eq!(vec![1, 2], row.get_checked::<_, Vec>(0).unwrap()); - assert_eq!("text", row.get_checked::<_, String>(1).unwrap()); - assert_eq!(1, row.get_checked::<_, c_int>(2).unwrap()); - assert!((1.5 - row.get_checked::<_, c_double>(3).unwrap()).abs() < EPSILON); - assert!(row.get_checked::<_, Option>(4).unwrap().is_none()); - assert!(row.get_checked::<_, Option>(4).unwrap().is_none()); - assert!(row.get_checked::<_, Option>(4).unwrap().is_none()); + assert_eq!(vec![1, 2], row.get::<_, Vec>(0).unwrap()); + assert_eq!("text", row.get::<_, String>(1).unwrap()); + assert_eq!(1, row.get::<_, c_int>(2).unwrap()); + assert!((1.5 - row.get::<_, c_double>(3).unwrap()).abs() < EPSILON); + assert!(row.get::<_, Option>(4).unwrap().is_none()); + assert!(row.get::<_, Option>(4).unwrap().is_none()); + assert!(row.get::<_, Option>(4).unwrap().is_none()); // check some invalid types // 0 is actually a blob (Vec) assert!(is_invalid_column_type( - row.get_checked::<_, c_int>(0).err().unwrap() + row.get::<_, c_int>(0).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, c_int>(0).err().unwrap() + row.get::<_, c_int>(0).err().unwrap() + )); + assert!(is_invalid_column_type(row.get::<_, i64>(0).err().unwrap())); + assert!(is_invalid_column_type( + row.get::<_, c_double>(0).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, i64>(0).err().unwrap() + row.get::<_, String>(0).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, c_double>(0).err().unwrap() + row.get::<_, time::Timespec>(0).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, String>(0).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, time::Timespec>(0).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, Option>(0).err().unwrap() + row.get::<_, Option>(0).err().unwrap() )); // 1 is actually a text (String) assert!(is_invalid_column_type( - row.get_checked::<_, c_int>(1).err().unwrap() + row.get::<_, c_int>(1).err().unwrap() + )); + assert!(is_invalid_column_type(row.get::<_, i64>(1).err().unwrap())); + assert!(is_invalid_column_type( + row.get::<_, c_double>(1).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, i64>(1).err().unwrap() + row.get::<_, Vec>(1).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, c_double>(1).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, Vec>(1).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, Option>(1).err().unwrap() + row.get::<_, Option>(1).err().unwrap() )); // 2 is actually an integer assert!(is_invalid_column_type( - row.get_checked::<_, String>(2).err().unwrap() + row.get::<_, String>(2).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, Vec>(2).err().unwrap() + row.get::<_, Vec>(2).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, Option>(2).err().unwrap() + row.get::<_, Option>(2).err().unwrap() )); // 3 is actually a float (c_double) assert!(is_invalid_column_type( - row.get_checked::<_, c_int>(3).err().unwrap() + row.get::<_, c_int>(3).err().unwrap() + )); + assert!(is_invalid_column_type(row.get::<_, i64>(3).err().unwrap())); + assert!(is_invalid_column_type( + row.get::<_, String>(3).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, i64>(3).err().unwrap() + row.get::<_, Vec>(3).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, String>(3).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, Vec>(3).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, Option>(3).err().unwrap() + row.get::<_, Option>(3).err().unwrap() )); // 4 is actually NULL assert!(is_invalid_column_type( - row.get_checked::<_, c_int>(4).err().unwrap() + row.get::<_, c_int>(4).err().unwrap() + )); + assert!(is_invalid_column_type(row.get::<_, i64>(4).err().unwrap())); + assert!(is_invalid_column_type( + row.get::<_, c_double>(4).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, i64>(4).err().unwrap() + row.get::<_, String>(4).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, c_double>(4).err().unwrap() + row.get::<_, Vec>(4).err().unwrap() )); assert!(is_invalid_column_type( - row.get_checked::<_, String>(4).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, Vec>(4).err().unwrap() - )); - assert!(is_invalid_column_type( - row.get_checked::<_, time::Timespec>(4).err().unwrap() + row.get::<_, time::Timespec>(4).err().unwrap() )); } @@ -360,19 +352,16 @@ mod test { let mut rows = stmt.query(NO_PARAMS).unwrap(); let row = rows.next().unwrap().unwrap(); - assert_eq!( - Value::Blob(vec![1, 2]), - row.get_checked::<_, Value>(0).unwrap() - ); + assert_eq!(Value::Blob(vec![1, 2]), row.get::<_, Value>(0).unwrap()); assert_eq!( Value::Text(String::from("text")), - row.get_checked::<_, Value>(1).unwrap() + row.get::<_, Value>(1).unwrap() ); - assert_eq!(Value::Integer(1), row.get_checked::<_, Value>(2).unwrap()); - match row.get_checked::<_, Value>(3).unwrap() { + assert_eq!(Value::Integer(1), row.get::<_, Value>(2).unwrap()); + match row.get::<_, Value>(3).unwrap() { Value::Real(val) => assert!((1.5 - val).abs() < EPSILON), x => panic!("Invalid Value {:?}", x), } - assert_eq!(Value::Null, row.get_checked::<_, Value>(4).unwrap()); + assert_eq!(Value::Null, row.get::<_, Value>(4).unwrap()); } } diff --git a/src/types/to_sql.rs b/src/types/to_sql.rs index 0d9a21d..ed5da10 100644 --- a/src/types/to_sql.rs +++ b/src/types/to_sql.rs @@ -229,7 +229,7 @@ mod test { let res = stmt .query_map(NO_PARAMS, |row| { - (row.get::<_, i128>(0), row.get::<_, String>(1)) + Ok((row.get::<_, i128>(0)?, row.get::<_, String>(1)?)) }) .unwrap() .collect::, _>>() diff --git a/src/vtab/csvtab.rs b/src/vtab/csvtab.rs index 67659f3..8e733f4 100644 --- a/src/vtab/csvtab.rs +++ b/src/vtab/csvtab.rs @@ -389,7 +389,7 @@ mod test { let mut rows = s.query(NO_PARAMS).unwrap(); let row = rows.next().unwrap().unwrap(); - assert_eq!(row.get::<_, i32>(0), 2); + assert_eq!(row.get_unwrap::<_, i32>(0), 2); } db.execute_batch("DROP TABLE vtab").unwrap(); } From 32881d7a762f0580f41efe2e3a78784d4add8491 Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 21 Feb 2019 21:14:55 +0100 Subject: [PATCH 14/15] Unify callback parameter signature --- src/pragma.rs | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/pragma.rs b/src/pragma.rs index 50302be..19c6791 100644 --- a/src/pragma.rs +++ b/src/pragma.rs @@ -171,18 +171,11 @@ impl Connection { f: F, ) -> Result where - F: FnOnce(Row<'_, '_>) -> Result, + F: FnOnce(&Row<'_, '_>) -> Result, { let mut query = Sql::new(); query.push_pragma(schema_name, pragma_name)?; - let mut stmt = self.prepare(&query)?; - let mut rows = stmt.query(NO_PARAMS)?; - if let Some(result_row) = rows.next() { - let row = result_row?; - f(row) - } else { - Err(Error::QueryReturnedNoRows) - } + self.query_row(&query, NO_PARAMS, f) } /// Query the current rows/values of `pragma_name`. @@ -276,7 +269,7 @@ impl Connection { f: F, ) -> Result where - F: FnOnce(Row<'_, '_>) -> Result, + F: FnOnce(&Row<'_, '_>) -> Result, { let mut sql = Sql::new(); sql.push_pragma(schema_name, pragma_name)?; @@ -285,14 +278,7 @@ impl Connection { // The two syntaxes yield identical results. sql.push_equal_sign(); sql.push_value(pragma_value)?; - let mut stmt = self.prepare(&sql)?; - let mut rows = stmt.query(NO_PARAMS)?; - if let Some(result_row) = rows.next() { - let row = result_row?; - f(row) - } else { - Err(Error::QueryReturnedNoRows) - } + self.query_row(&sql, NO_PARAMS, f) } } @@ -333,7 +319,7 @@ mod test { fn pragma_query_value() { let db = Connection::open_in_memory().unwrap(); let user_version: i32 = db - .pragma_query_value(None, "user_version", |row| row.get_checked(0)) + .pragma_query_value(None, "user_version", |row| row.get(0)) .unwrap(); assert_eq!(0, user_version); } @@ -359,7 +345,7 @@ mod test { let db = Connection::open_in_memory().unwrap(); let mut user_version = -1; db.pragma_query(None, "user_version", |row| { - user_version = row.get_checked(0)?; + user_version = row.get(0)?; Ok(()) }) .unwrap(); @@ -371,7 +357,7 @@ mod test { let db = Connection::open_in_memory().unwrap(); let mut user_version = -1; db.pragma_query(Some(DatabaseName::Main), "user_version", |row| { - user_version = row.get_checked(0)?; + user_version = row.get(0)?; Ok(()) }) .unwrap(); @@ -383,7 +369,7 @@ mod test { let db = Connection::open_in_memory().unwrap(); let mut columns = Vec::new(); db.pragma(None, "table_info", &"sqlite_master", |row| { - let column: String = row.get_checked(1)?; + let column: String = row.get(1)?; columns.push(column); Ok(()) }) @@ -401,7 +387,7 @@ mod test { while let Some(row) = rows.next() { let row = row.unwrap(); - let column: String = row.get(1); + let column: String = row.get(1).unwrap(); columns.push(column); } assert_eq!(5, columns.len()); @@ -417,7 +403,7 @@ mod test { fn pragma_update_and_check() { let db = Connection::open_in_memory().unwrap(); let journal_mode: String = db - .pragma_update_and_check(None, "journal_mode", &"OFF", |row| row.get_checked(0)) + .pragma_update_and_check(None, "journal_mode", &"OFF", |row| row.get(0)) .unwrap(); assert_eq!("off", &journal_mode); } From 04f900059dd59b567f68b812356c18cee432128b Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 26 Feb 2019 19:38:41 -0800 Subject: [PATCH 15/15] Avoid unnecessary copies/allocations when passing strings to sqlite --- Cargo.toml | 1 + src/context.rs | 15 +++++---------- src/inner_connection.rs | 16 ++++++---------- src/lib.rs | 33 +++++++++++++++++++++++++++++++++ src/statement.rs | 36 ++++++++++++------------------------ 5 files changed, 57 insertions(+), 44 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fb288ef..861c0f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -59,6 +59,7 @@ csv = { version = "1.0", optional = true } lazy_static = { version = "1.0", optional = true } byteorder = { version = "1.2", features = ["i128"], optional = true } fallible-streaming-iterator = { version = "0.1", optional = true } +memchr = "2.2.0" [dev-dependencies] tempdir = "0.3" diff --git a/src/context.rs b/src/context.rs index a7bb468..ad0a3ad 100644 --- a/src/context.rs +++ b/src/context.rs @@ -7,7 +7,7 @@ use std::rc::Rc; use crate::ffi; use crate::ffi::sqlite3_context; -use crate::str_to_cstring; +use crate::str_for_sqlite; use crate::types::{ToSqlOutput, ValueRef}; #[cfg(feature = "array")] use crate::vtab::array::{free_array, ARRAY_TYPE}; @@ -38,25 +38,20 @@ pub(crate) unsafe fn set_result(ctx: *mut sqlite3_context, result: &ToSqlOutput< ValueRef::Real(r) => ffi::sqlite3_result_double(ctx, r), ValueRef::Text(s) => { let length = s.len(); - if length > ::std::i32::MAX as usize { + if length > c_int::max_value() as usize { ffi::sqlite3_result_error_toobig(ctx); } else { - let c_str = match str_to_cstring(s) { + let (c_str, len, destructor) = match str_for_sqlite(s) { Ok(c_str) => c_str, // TODO sqlite3_result_error Err(_) => return ffi::sqlite3_result_error_code(ctx, ffi::SQLITE_MISUSE), }; - let destructor = if length > 0 { - ffi::SQLITE_TRANSIENT() - } else { - ffi::SQLITE_STATIC() - }; - ffi::sqlite3_result_text(ctx, c_str.as_ptr(), length as c_int, destructor); + ffi::sqlite3_result_text(ctx, c_str, len, destructor); } } ValueRef::Blob(b) => { let length = b.len(); - if length > ::std::i32::MAX as usize { + if length > c_int::max_value() as usize { ffi::sqlite3_result_error_toobig(ctx); } else if length == 0 { ffi::sqlite3_result_zeroblob(ctx, 0) diff --git a/src/inner_connection.rs b/src/inner_connection.rs index 499dcd6..421a131 100644 --- a/src/inner_connection.rs +++ b/src/inner_connection.rs @@ -9,7 +9,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, Once, ONCE_INIT}; use super::ffi; -use super::str_to_cstring; +use super::{str_to_cstring, str_for_sqlite}; use super::{Connection, InterruptHandle, OpenFlags, Result}; use crate::error::{error_from_handle, error_from_sqlite_code, Error}; use crate::raw_statement::RawStatement; @@ -207,20 +207,16 @@ impl InnerConnection { } pub fn prepare<'a>(&mut self, conn: &'a Connection, sql: &str) -> Result> { - if sql.len() >= ::std::i32::MAX as usize { - return Err(error_from_sqlite_code(ffi::SQLITE_TOOBIG, None)); - } let mut c_stmt: *mut ffi::sqlite3_stmt = unsafe { mem::uninitialized() }; - let c_sql = str_to_cstring(sql)?; - let len_with_nul = (sql.len() + 1) as c_int; + let (c_sql, len, _) = str_for_sqlite(sql)?; let r = unsafe { if cfg!(feature = "unlock_notify") { let mut rc; loop { rc = ffi::sqlite3_prepare_v2( self.db(), - c_sql.as_ptr(), - len_with_nul, + c_sql, + len, &mut c_stmt, ptr::null_mut(), ); @@ -236,8 +232,8 @@ impl InnerConnection { } else { ffi::sqlite3_prepare_v2( self.db(), - c_sql.as_ptr(), - len_with_nul, + c_sql, + len, &mut c_stmt, ptr::null_mut(), ) diff --git a/src/lib.rs b/src/lib.rs index a06a25f..082a166 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -237,6 +237,39 @@ fn str_to_cstring(s: &str) -> Result { Ok(CString::new(s)?) } +/// Returns `Ok((string ptr, len as c_int, SQLITE_STATIC | SQLITE_TRANSIENT))` +/// normally. +/// Returns errors if the string has embedded nuls or is too large for sqlite. +/// The `sqlite3_destructor_type` item is always `SQLITE_TRANSIENT` unless +/// the string was empty (in which case it's `SQLITE_STATIC`, and the ptr is +/// static). +fn str_for_sqlite(s: &str) -> Result<(*const c_char, c_int, ffi::sqlite3_destructor_type)> { + let len = len_as_c_int(s.len())?; + if memchr::memchr(0, s.as_bytes()).is_none() { + let (ptr, dtor_info) = if len != 0 { + (s.as_ptr() as *const c_char, ffi::SQLITE_TRANSIENT()) + } else { + // Return a pointer guaranteed to live forever + ("".as_ptr() as *const c_char, ffi::SQLITE_STATIC()) + }; + Ok((ptr, len, dtor_info)) + } else { + // There's an embedded nul, so we fabricate a NulError. + let e = CString::new(s); + Err(Error::NulError(e.unwrap_err())) + } +} + +// Helper to cast to c_int safely, returning the correct error type if the cast +// failed. +fn len_as_c_int(len: usize) -> Result { + if len >= (c_int::max_value() as usize) { + Err(Error::SqliteFailure(ffi::Error::new(ffi::SQLITE_TOOBIG), None)) + } else { + Ok(len as c_int) + } +} + fn path_to_cstring(p: &Path) -> Result { let s = p.to_str().ok_or_else(|| Error::InvalidPath(p.to_owned()))?; str_to_cstring(s) diff --git a/src/statement.rs b/src/statement.rs index aa07ea7..241b21f 100644 --- a/src/statement.rs +++ b/src/statement.rs @@ -7,7 +7,7 @@ use std::slice::from_raw_parts; use std::{convert, fmt, mem, ptr, result, str}; use super::ffi; -use super::str_to_cstring; +use super::{str_to_cstring, str_for_sqlite, len_as_c_int}; use super::{ AndThenRows, Connection, Error, MappedRows, RawStatement, Result, Row, Rows, ValueRef, }; @@ -506,37 +506,25 @@ impl Statement<'_> { ValueRef::Integer(i) => unsafe { ffi::sqlite3_bind_int64(ptr, col as c_int, i) }, ValueRef::Real(r) => unsafe { ffi::sqlite3_bind_double(ptr, col as c_int, r) }, ValueRef::Text(s) => unsafe { - let length = s.len(); - if length > ::std::i32::MAX as usize { - ffi::SQLITE_TOOBIG - } else { - let c_str = str_to_cstring(s)?; - let destructor = if length > 0 { - ffi::SQLITE_TRANSIENT() - } else { - ffi::SQLITE_STATIC() - }; - ffi::sqlite3_bind_text( - ptr, - col as c_int, - c_str.as_ptr(), - length as c_int, - destructor, - ) - } + let (c_str, len, destructor) = str_for_sqlite(s)?; + ffi::sqlite3_bind_text( + ptr, + col as c_int, + c_str, + len, + destructor, + ) }, ValueRef::Blob(b) => unsafe { - let length = b.len(); - if length > ::std::i32::MAX as usize { - ffi::SQLITE_TOOBIG - } else if length == 0 { + let length = len_as_c_int(b.len())?; + if length == 0 { ffi::sqlite3_bind_zeroblob(ptr, col as c_int, 0) } else { ffi::sqlite3_bind_blob( ptr, col as c_int, b.as_ptr() as *const c_void, - length as c_int, + length, ffi::SQLITE_TRANSIENT(), ) }