From e4926ac0d71ac1ffaf27b8bd48163d3c3de6e85e Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 25 May 2016 22:57:43 -0400 Subject: [PATCH 01/28] Rework ToSql to be implementable without `unsafe`. --- src/blob.rs | 7 +-- src/functions.rs | 4 +- src/lib.rs | 63 +++++++++++++++++---- src/named_params.rs | 7 +-- src/types/chrono.rs | 22 +++----- src/types/mod.rs | 61 +++++++++++++++++++- src/types/serde_json.rs | 19 +++---- src/types/time.rs | 11 ++-- src/types/to_sql.rs | 122 +++++++++++++++++++--------------------- src/types/value_ref.rs | 12 ++++ 10 files changed, 206 insertions(+), 122 deletions(-) diff --git a/src/blob.rs b/src/blob.rs index 548e5c9..5bac455 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -52,10 +52,9 @@ use std::io; use std::cmp::min; use std::mem; use std::ptr; -use libc::c_int; use super::ffi; -use super::types::ToSql; +use super::types::{ToSql, ToSqlOutput}; use {Result, Connection, DatabaseName}; /// Handle to an open BLOB. @@ -244,9 +243,9 @@ impl<'conn> Drop for Blob<'conn> { pub struct ZeroBlob(pub i32); impl ToSql for ZeroBlob { - unsafe fn bind_parameter(&self, stmt: *mut ffi::sqlite3_stmt, col: c_int) -> c_int { + fn to_sql(&self) -> Result { let ZeroBlob(length) = *self; - ffi::sqlite3_bind_zeroblob(stmt, col, length) + Ok(ToSqlOutput::ZeroBlob(length)) } } diff --git a/src/functions.rs b/src/functions.rs index 32e4d1e..c80b407 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -182,7 +182,7 @@ impl<'a> ValueRef<'a> { ValueRef::Blob(from_raw_parts(blob as *const u8, len as usize)) } - _ => unreachable!("sqlite3_value_type returned invalid value") + _ => unreachable!("sqlite3_value_type returned invalid value"), } } } @@ -219,7 +219,7 @@ impl<'a> Context<'a> { let value = unsafe { ValueRef::from_value(arg) }; FromSql::column_result(value).map_err(|err| match err { Error::InvalidColumnType => Error::InvalidFunctionParameterType, - _ => err + _ => err, }) } diff --git a/src/lib.rs b/src/lib.rs index 8d84890..f8c6830 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -73,9 +73,9 @@ use std::cell::RefCell; use std::ffi::{CStr, CString}; use std::result; use std::str; -use libc::{c_int, c_char}; +use libc::{c_int, c_char, c_void}; -use types::{ToSql, FromSql, ValueRef}; +use types::{ToSql, ToSqlOutput, FromSql, ValueRef}; use error::{error_from_sqlite_code, error_from_handle}; use raw_statement::RawStatement; use cache::StatementCache; @@ -874,6 +874,54 @@ impl<'conn> Statement<'conn> { self.finalize_() } + fn bind_parameter(&self, param: &ToSql, col: c_int) -> Result<()> { + // This should be + // let value = try!(param.to_sql()); + // but that hits a bug in the Rust compiler around re-exported + // trait visibility. It's fixed in 1.9. + let value = try!(ToSql::to_sql(param)); + + let ptr = unsafe { self.stmt.ptr() }; + let value = match value { + ToSqlOutput::Borrowed(v) => v, + ToSqlOutput::Owned(ref v) => ValueRef::from(v), + + #[cfg(feature = "blob")] + ToSqlOutput::ZeroBlob(len) => { + return self.conn.decode_result(unsafe { ffi::sqlite3_bind_zeroblob(ptr, col, len) }); + } + }; + self.conn.decode_result(match value { + ValueRef::Null => unsafe { ffi::sqlite3_bind_null(ptr, col) }, + ValueRef::Integer(i) => unsafe { ffi::sqlite3_bind_int64(ptr, col, i) }, + ValueRef::Real(r) => unsafe { ffi::sqlite3_bind_double(ptr, col, r) }, + ValueRef::Text(ref s) => unsafe { + let length = s.len(); + if length > ::std::i32::MAX as usize { + ffi::SQLITE_TOOBIG + } else { + let c_str = try!(str_to_cstring(s)); + let destructor = if length > 0 { + ffi::SQLITE_TRANSIENT() + } else { + ffi::SQLITE_STATIC() + }; + ffi::sqlite3_bind_text(ptr, col, c_str.as_ptr(), length as c_int, destructor) + } + }, + ValueRef::Blob(ref b) => unsafe { + let length = b.len(); + if length > ::std::i32::MAX as usize { + ffi::SQLITE_TOOBIG + } else if length == 0 { + ffi::sqlite3_bind_zeroblob(ptr, col, 0) + } else { + ffi::sqlite3_bind_blob(ptr, col, b.as_ptr() as *const c_void, length as c_int, ffi::SQLITE_TRANSIENT()) + } + }, + }) + } + 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 {}", @@ -881,14 +929,7 @@ impl<'conn> Statement<'conn> { params.len()); for (i, p) in params.iter().enumerate() { - try!(unsafe { - self.conn.decode_result( - // This should be - // `p.bind_parameter(self.stmt.ptr(), (i + 1) as c_int)` - // but that doesn't compile until Rust 1.9 due to a compiler bug. - ToSql::bind_parameter(*p, self.stmt.ptr(), (i + 1) as c_int) - ) - }); + try!(self.bind_parameter(*p, (i + 1) as c_int)); } Ok(()) @@ -1128,7 +1169,7 @@ impl<'a> ValueRef<'a> { ValueRef::Blob(from_raw_parts(blob as *const u8, len as usize)) } - _ => unreachable!("sqlite3_column_type returned invalid value") + _ => unreachable!("sqlite3_column_type returned invalid value"), } } } diff --git a/src/named_params.rs b/src/named_params.rs index d864736..7529215 100644 --- a/src/named_params.rs +++ b/src/named_params.rs @@ -204,12 +204,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 { - // This should be - // `value.bind_parameter(self.stmt.ptr(), i)` - // but that doesn't compile until Rust 1.9 due to a compiler bug. - ToSql::bind_parameter(value, self.stmt.ptr(), i) - })); + try!(self.bind_parameter(value, i)); } else { return Err(Error::InvalidParameterName(name.into())); } diff --git a/src/types/chrono.rs b/src/types/chrono.rs index 3d17dd6..525c5bc 100644 --- a/src/types/chrono.rs +++ b/src/types/chrono.rs @@ -4,18 +4,15 @@ extern crate chrono; use std::borrow::Cow; use self::chrono::{NaiveDate, NaiveTime, NaiveDateTime, DateTime, TimeZone, UTC, Local}; -use libc::c_int; use {Error, Result}; -use types::{FromSql, ToSql, ValueRef}; - -use ffi::sqlite3_stmt; +use types::{FromSql, ToSql, ToSqlOutput, ValueRef}; /// ISO 8601 calendar date without timezone => "YYYY-MM-DD" impl ToSql for NaiveDate { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { + fn to_sql(&self) -> Result { let date_str = self.format("%Y-%m-%d").to_string(); - date_str.bind_parameter(stmt, col) + Ok(ToSqlOutput::from(date_str)) } } @@ -31,9 +28,9 @@ impl FromSql for NaiveDate { /// ISO 8601 time without timezone => "HH:MM:SS.SSS" impl ToSql for NaiveTime { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { + fn to_sql(&self) -> Result { let date_str = self.format("%H:%M:%S%.f").to_string(); - date_str.bind_parameter(stmt, col) + Ok(ToSqlOutput::from(date_str)) } } @@ -56,9 +53,9 @@ impl FromSql for NaiveTime { /// ISO 8601 combined date and time without timezone => "YYYY-MM-DD HH:MM:SS.SSS" impl ToSql for NaiveDateTime { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { + fn to_sql(&self) -> Result { let date_str = self.format("%Y-%m-%dT%H:%M:%S%.f").to_string(); - date_str.bind_parameter(stmt, col) + Ok(ToSqlOutput::from(date_str)) } } @@ -83,9 +80,8 @@ impl FromSql for NaiveDateTime { /// Date and time with time zone => UTC RFC3339 timestamp ("YYYY-MM-DDTHH:MM:SS.SSS+00:00"). impl ToSql for DateTime { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - let utc_dt = self.with_timezone(&UTC); - utc_dt.to_rfc3339().bind_parameter(stmt, col) + fn to_sql(&self) -> Result { + Ok(ToSqlOutput::from(self.with_timezone(&UTC).to_rfc3339())) } } diff --git a/src/types/mod.rs b/src/types/mod.rs index 758eb5d..31afcf7 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -50,10 +50,8 @@ //! `FromSql` for the cases where you want to know if a value was NULL (which gets translated to //! `None`). -pub use ffi::sqlite3_stmt; - pub use self::from_sql::FromSql; -pub use self::to_sql::ToSql; +pub use self::to_sql::{ToSql, ToSqlOutput}; pub use self::value_ref::ValueRef; mod value_ref; @@ -102,6 +100,42 @@ pub enum Value { Blob(Vec), } +impl From for Value { + fn from(_: Null) -> Value { + Value::Null + } +} + +impl From for Value { + fn from(i: i32) -> Value { + Value::Integer(i as i64) + } +} + +impl From for Value { + fn from(i: i64) -> Value { + Value::Integer(i) + } +} + +impl From for Value { + fn from(f: f64) -> Value { + Value::Real(f) + } +} + +impl From for Value { + fn from(s: String) -> Value { + Value::Text(s) + } +} + +impl From> for Value { + fn from(v: Vec) -> Value { + Value::Blob(v) + } +} + #[cfg(test)] #[cfg_attr(feature="clippy", allow(similar_names))] mod test { @@ -111,6 +145,7 @@ mod test { use Error; use libc::{c_int, c_double}; use std::f64::EPSILON; + use super::Value; fn checked_memory_handle() -> Connection { let db = Connection::open_in_memory().unwrap(); @@ -133,6 +168,17 @@ mod test { fn test_str() { let db = checked_memory_handle(); + let s = "hello, world!"; + db.execute("INSERT INTO foo(t) VALUES (?)", &[&s]).unwrap(); + + let from: String = db.query_row("SELECT t FROM foo", &[], |r| r.get(0)).unwrap(); + assert_eq!(from, s); + } + + #[test] + fn test_string() { + let db = checked_memory_handle(); + let s = "hello, world!"; db.execute("INSERT INTO foo(t) VALUES (?)", &[&s.to_owned()]).unwrap(); @@ -140,6 +186,15 @@ mod test { assert_eq!(from, s); } + #[test] + fn test_value() { + let db = checked_memory_handle(); + + db.execute("INSERT INTO foo(i) VALUES (?)", &[&Value::Integer(10)]).unwrap(); + + assert_eq!(10i64, db.query_row("SELECT i FROM foo", &[], |r| r.get(0)).unwrap()); + } + #[test] fn test_option() { let db = checked_memory_handle(); diff --git a/src/types/serde_json.rs b/src/types/serde_json.rs index 937f914..e000c4d 100644 --- a/src/types/serde_json.rs +++ b/src/types/serde_json.rs @@ -1,19 +1,15 @@ //! `ToSql` and `FromSql` implementation for JSON `Value`. extern crate serde_json; -use libc::c_int; use self::serde_json::Value; use {Error, Result}; -use types::{FromSql, ToSql, ValueRef}; - -use ffi::sqlite3_stmt; +use types::{FromSql, ToSql, ToSqlOutput, ValueRef}; /// Serialize JSON `Value` to text. impl ToSql for Value { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - let s = serde_json::to_string(self).unwrap(); - s.bind_parameter(stmt, col) + fn to_sql(&self) -> Result { + Ok(ToSqlOutput::from(serde_json::to_string(self).unwrap())) } } @@ -21,10 +17,11 @@ impl ToSql for Value { impl FromSql for Value { fn column_result(value: ValueRef) -> Result { match value { - ValueRef::Text(ref s) => serde_json::from_str(s), - ValueRef::Blob(ref b) => serde_json::from_slice(b), - _ => return Err(Error::InvalidColumnType), - }.map_err(|err| Error::FromSqlConversionFailure(Box::new(err))) + ValueRef::Text(ref s) => serde_json::from_str(s), + ValueRef::Blob(ref b) => serde_json::from_slice(b), + _ => return Err(Error::InvalidColumnType), + } + .map_err(|err| Error::FromSqlConversionFailure(Box::new(err))) } } diff --git a/src/types/time.rs b/src/types/time.rs index 1f1a818..bbc82bd 100644 --- a/src/types/time.rs +++ b/src/types/time.rs @@ -1,17 +1,14 @@ extern crate time; -use libc::c_int; use {Error, Result}; -use types::{FromSql, ToSql, ValueRef}; - -use ffi::sqlite3_stmt; +use types::{FromSql, ToSql, ToSqlOutput, ValueRef}; const SQLITE_DATETIME_FMT: &'static str = "%Y-%m-%d %H:%M:%S"; impl ToSql for time::Timespec { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - let time_str = time::at_utc(*self).strftime(SQLITE_DATETIME_FMT).unwrap().to_string(); - time_str.bind_parameter(stmt, col) + fn to_sql(&self) -> Result { + let time_string = time::at_utc(*self).strftime(SQLITE_DATETIME_FMT).unwrap().to_string(); + Ok(ToSqlOutput::from(time_string)) } } diff --git a/src/types/to_sql.rs b/src/types/to_sql.rs index 99293b5..bff9a0a 100644 --- a/src/types/to_sql.rs +++ b/src/types/to_sql.rs @@ -1,95 +1,87 @@ -use std::mem; +use super::{Null, Value, ValueRef}; +use ::Result; -use libc::{c_double, c_int}; +pub enum ToSqlOutput<'a> { + Borrowed(ValueRef<'a>), + Owned(Value), -use super::Null; -use ::{ffi, str_to_cstring}; -use ffi::sqlite3_stmt; + #[cfg(feature = "blob")] + ZeroBlob(i32), +} + +impl<'a, T: ?Sized> From<&'a T> for ToSqlOutput<'a> where &'a T: Into> { + fn from(t: &'a T) -> Self { + ToSqlOutput::Borrowed(t.into()) + } +} + +impl<'a, T: Into> From for ToSqlOutput<'a> { + fn from(t: T) -> Self { + ToSqlOutput::Owned(t.into()) + } +} /// A trait for types that can be converted into SQLite values. pub trait ToSql { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int; + fn to_sql(&self) -> Result; } -macro_rules! raw_to_impl( - ($t:ty, $f:ident) => ( +// We should be able to use a generic impl like this: +// +// impl ToSql for T where T: Into { +// fn to_sql(&self) -> Result { +// Ok(ToSqlOutput::from((*self).into())) +// } +// } +// +// instead of the following macro, but this runs afoul of +// https://github.com/rust-lang/rust/issues/30191 and reports conflicting +// implementations even when there aren't any. + +macro_rules! to_sql_self( + ($t:ty) => ( impl ToSql for $t { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - ffi::$f(stmt, col, *self) + fn to_sql(&self) -> Result { + Ok(ToSqlOutput::from(*self)) } } ) ); -raw_to_impl!(c_int, sqlite3_bind_int); // i32 -raw_to_impl!(i64, sqlite3_bind_int64); -raw_to_impl!(c_double, sqlite3_bind_double); +to_sql_self!(Null); +to_sql_self!(i32); +to_sql_self!(i64); +to_sql_self!(f64); -impl ToSql for bool { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - if *self { - ffi::sqlite3_bind_int(stmt, col, 1) - } else { - ffi::sqlite3_bind_int(stmt, col, 0) - } - } -} - -impl<'a> ToSql for &'a str { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - let length = self.len(); - if length > ::std::i32::MAX as usize { - return ffi::SQLITE_TOOBIG; - } - match str_to_cstring(self) { - Ok(c_str) => { - ffi::sqlite3_bind_text(stmt, - col, - c_str.as_ptr(), - length as c_int, - ffi::SQLITE_TRANSIENT()) - } - Err(_) => ffi::SQLITE_MISUSE, - } +impl<'a, T: ?Sized> ToSql for &'a T where &'a T: Into> { + fn to_sql(&self) -> Result { + Ok(ToSqlOutput::from((*self).into())) } } impl ToSql for String { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - (&self[..]).bind_parameter(stmt, col) - } -} - -impl<'a> ToSql for &'a [u8] { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - if self.len() > ::std::i32::MAX as usize { - return ffi::SQLITE_TOOBIG; - } - ffi::sqlite3_bind_blob(stmt, - col, - mem::transmute(self.as_ptr()), - self.len() as c_int, - ffi::SQLITE_TRANSIENT()) + fn to_sql(&self) -> Result { + Ok(ToSqlOutput::from(self.as_str())) } } impl ToSql for Vec { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - (&self[..]).bind_parameter(stmt, col) + fn to_sql(&self) -> Result { + Ok(ToSqlOutput::from(self.as_slice())) + } +} + +impl ToSql for Value { + fn to_sql(&self) -> Result { + Ok(ToSqlOutput::from(self)) } } impl ToSql for Option { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { + fn to_sql(&self) -> Result { match *self { - None => ffi::sqlite3_bind_null(stmt, col), - Some(ref t) => t.bind_parameter(stmt, col), + None => Ok(ToSqlOutput::from(Null)), + Some(ref t) => t.to_sql(), } } } - -impl ToSql for Null { - unsafe fn bind_parameter(&self, stmt: *mut sqlite3_stmt, col: c_int) -> c_int { - ffi::sqlite3_bind_null(stmt, col) - } -} diff --git a/src/types/value_ref.rs b/src/types/value_ref.rs index aa2d3b8..819a632 100644 --- a/src/types/value_ref.rs +++ b/src/types/value_ref.rs @@ -70,6 +70,18 @@ impl<'a> From> for Value { } } +impl<'a> From<&'a str> for ValueRef<'a> { + fn from(s: &str) -> ValueRef { + ValueRef::Text(s) + } +} + +impl<'a> From<&'a [u8]> for ValueRef<'a> { + fn from(s: &[u8]) -> ValueRef { + ValueRef::Blob(s) + } +} + impl<'a> From<&'a Value> for ValueRef<'a> { fn from(value: &'a Value) -> ValueRef<'a> { match *value { From 912582653479b474e317fd509457e35d98cff454 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 25 May 2016 23:30:34 -0400 Subject: [PATCH 02/28] Move types::Value into its own module. --- src/types/mod.rs | 56 ++-------------------------------------------- src/types/value.rs | 55 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 54 deletions(-) create mode 100644 src/types/value.rs diff --git a/src/types/mod.rs b/src/types/mod.rs index 31afcf7..92ca3fb 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -52,8 +52,10 @@ pub use self::from_sql::FromSql; pub use self::to_sql::{ToSql, ToSqlOutput}; +pub use self::value::Value; pub use self::value_ref::ValueRef; +mod value; mod value_ref; mod from_sql; mod to_sql; @@ -82,60 +84,6 @@ mod serde_json; #[derive(Copy,Clone)] pub struct Null; -/// Owning [dynamic type value](http://sqlite.org/datatype3.html). Value's type is typically -/// dictated by SQLite (not by the caller). -/// -/// See [`ValueRef`](enum.ValueRef.html) for a non-owning dynamic type value. -#[derive(Clone,Debug,PartialEq)] -pub enum Value { - /// The value is a `NULL` value. - Null, - /// The value is a signed integer. - Integer(i64), - /// The value is a floating point number. - Real(f64), - /// The value is a text string. - Text(String), - /// The value is a blob of data - Blob(Vec), -} - -impl From for Value { - fn from(_: Null) -> Value { - Value::Null - } -} - -impl From for Value { - fn from(i: i32) -> Value { - Value::Integer(i as i64) - } -} - -impl From for Value { - fn from(i: i64) -> Value { - Value::Integer(i) - } -} - -impl From for Value { - fn from(f: f64) -> Value { - Value::Real(f) - } -} - -impl From for Value { - fn from(s: String) -> Value { - Value::Text(s) - } -} - -impl From> for Value { - fn from(v: Vec) -> Value { - Value::Blob(v) - } -} - #[cfg(test)] #[cfg_attr(feature="clippy", allow(similar_names))] mod test { diff --git a/src/types/value.rs b/src/types/value.rs new file mode 100644 index 0000000..2776b98 --- /dev/null +++ b/src/types/value.rs @@ -0,0 +1,55 @@ +use super::Null; + +/// Owning [dynamic type value](http://sqlite.org/datatype3.html). Value's type is typically +/// dictated by SQLite (not by the caller). +/// +/// See [`ValueRef`](enum.ValueRef.html) for a non-owning dynamic type value. +#[derive(Clone,Debug,PartialEq)] +pub enum Value { + /// The value is a `NULL` value. + Null, + /// The value is a signed integer. + Integer(i64), + /// The value is a floating point number. + Real(f64), + /// The value is a text string. + Text(String), + /// The value is a blob of data + Blob(Vec), +} + +impl From for Value { + fn from(_: Null) -> Value { + Value::Null + } +} + +impl From for Value { + fn from(i: i32) -> Value { + Value::Integer(i as i64) + } +} + +impl From for Value { + fn from(i: i64) -> Value { + Value::Integer(i) + } +} + +impl From for Value { + fn from(f: f64) -> Value { + Value::Real(f) + } +} + +impl From for Value { + fn from(s: String) -> Value { + Value::Text(s) + } +} + +impl From> for Value { + fn from(v: Vec) -> Value { + Value::Blob(v) + } +} From 13bff6fab6010719abca44c176a6bc3a1b806678 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 May 2016 00:05:00 -0400 Subject: [PATCH 03/28] Add ToSql impl for `bool`. --- src/types/to_sql.rs | 1 + src/types/value.rs | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/types/to_sql.rs b/src/types/to_sql.rs index bff9a0a..1680f41 100644 --- a/src/types/to_sql.rs +++ b/src/types/to_sql.rs @@ -49,6 +49,7 @@ macro_rules! to_sql_self( ); to_sql_self!(Null); +to_sql_self!(bool); to_sql_self!(i32); to_sql_self!(i64); to_sql_self!(f64); diff --git a/src/types/value.rs b/src/types/value.rs index 2776b98..a6d2339 100644 --- a/src/types/value.rs +++ b/src/types/value.rs @@ -24,6 +24,12 @@ impl From for Value { } } +impl From for Value { + fn from(i: bool) -> Value { + Value::Integer(i as i64) + } +} + impl From for Value { fn from(i: i32) -> Value { Value::Integer(i as i64) From 6467815d02af4363b1764b21e7213bc6ddb1bf4a Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 May 2016 00:06:53 -0400 Subject: [PATCH 04/28] Replace functions::ToResult with ToSql. --- src/functions.rs | 207 ++++++++++++++++++----------------------------- 1 file changed, 80 insertions(+), 127 deletions(-) diff --git a/src/functions.rs b/src/functions.rs index c80b407..a489475 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -54,7 +54,7 @@ use std::ffi::CStr; use std::mem; use std::ptr; use std::slice; -use libc::{c_int, c_double, c_char, c_void}; +use libc::{c_int, c_char, c_void}; use ffi; pub use ffi::sqlite3_context; @@ -62,100 +62,73 @@ pub use ffi::sqlite3_value; pub use ffi::sqlite3_value_type; pub use ffi::sqlite3_value_numeric_type; -use types::{Null, FromSql, ValueRef}; +use types::{ToSql, ToSqlOutput, FromSql, ValueRef}; use {Result, Error, Connection, str_to_cstring, InnerConnection}; -/// A trait for types that can be converted into the result of an SQL function. -pub trait ToResult { - unsafe fn set_result(&self, ctx: *mut sqlite3_context); +fn set_result<'a>(ctx: *mut sqlite3_context, result: &ToSqlOutput<'a>) { + let value = match *result { + ToSqlOutput::Borrowed(v) => v, + ToSqlOutput::Owned(ref v) => ValueRef::from(v), + + #[cfg(feature = "blob")] + ToSqlOutput::ZeroBlob(len) => { + return unsafe { ffi::sqlite3_result_zeroblob(ctx, len) }; + } + }; + + match value { + ValueRef::Null => unsafe { ffi::sqlite3_result_null(ctx) }, + ValueRef::Integer(i) => unsafe { ffi::sqlite3_result_int64(ctx, i) }, + ValueRef::Real(r) => unsafe { ffi::sqlite3_result_double(ctx, r) }, + ValueRef::Text(ref s) => unsafe { + let length = s.len(); + if length > ::std::i32::MAX as usize { + ffi::sqlite3_result_error_toobig(ctx); + } else { + let c_str = match str_to_cstring(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); + } + }, + ValueRef::Blob(ref b) => unsafe { + let length = b.len(); + if length > ::std::i32::MAX as usize { + ffi::sqlite3_result_error_toobig(ctx); + } else if length == 0 { + ffi::sqlite3_result_zeroblob(ctx, 0) + } else { + ffi::sqlite3_result_blob(ctx, b.as_ptr() as *const c_void, length as c_int, ffi::SQLITE_TRANSIENT()); + } + }, + } } -macro_rules! raw_to_impl( - ($t:ty, $f:ident) => ( - impl ToResult for $t { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - ffi::$f(ctx, *self) +unsafe fn report_error(ctx: *mut sqlite3_context, err: &Error) { + match err { + &Error::SqliteFailure(ref err, ref s) => { + ffi::sqlite3_result_error_code(ctx, err.extended_code); + if let Some(Ok(cstr)) = s.as_ref().map(|s| str_to_cstring(s)) { + ffi::sqlite3_result_error(ctx, cstr.as_ptr(), -1); } } - ) -); - -raw_to_impl!(c_int, sqlite3_result_int); -raw_to_impl!(i64, sqlite3_result_int64); -raw_to_impl!(c_double, sqlite3_result_double); - -impl<'a> ToResult for bool { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - if *self { - ffi::sqlite3_result_int(ctx, 1) - } else { - ffi::sqlite3_result_int(ctx, 0) - } - } -} - - -impl<'a> ToResult for &'a str { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - let length = self.len(); - if length > ::std::i32::MAX as usize { - ffi::sqlite3_result_error_toobig(ctx); - return; - } - match str_to_cstring(self) { - Ok(c_str) => { - ffi::sqlite3_result_text(ctx, - c_str.as_ptr(), - length as c_int, - ffi::SQLITE_TRANSIENT()) + _ => { + ffi::sqlite3_result_error_code(ctx, ffi::SQLITE_CONSTRAINT_FUNCTION); + if let Ok(cstr) = str_to_cstring(err.description()) { + ffi::sqlite3_result_error(ctx, cstr.as_ptr(), -1); } - // TODO sqlite3_result_error - Err(_) => ffi::sqlite3_result_error_code(ctx, ffi::SQLITE_MISUSE), } } } -impl ToResult for String { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - (&self[..]).set_result(ctx) - } -} - -impl<'a> ToResult for &'a [u8] { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - if self.len() > ::std::i32::MAX as usize { - ffi::sqlite3_result_error_toobig(ctx); - return; - } - ffi::sqlite3_result_blob(ctx, - mem::transmute(self.as_ptr()), - self.len() as c_int, - ffi::SQLITE_TRANSIENT()) - } -} - -impl ToResult for Vec { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - (&self[..]).set_result(ctx) - } -} - -impl ToResult for Option { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - match *self { - None => ffi::sqlite3_result_null(ctx), - Some(ref t) => t.set_result(ctx), - } - } -} - -impl ToResult for Null { - unsafe fn set_result(&self, ctx: *mut sqlite3_context) { - ffi::sqlite3_result_null(ctx) - } -} - impl<'a> ValueRef<'a> { unsafe fn from_value(value: *mut sqlite3_value) -> ValueRef<'a> { use std::slice::from_raw_parts; @@ -259,7 +232,7 @@ impl<'a> Context<'a> { /// `A` is the type of the aggregation context and `T` is the type of the final result. /// Implementations should be stateless. pub trait Aggregate - where T: ToResult + where T: ToSql { /// Initializes the aggregation context. Will be called prior to the first call /// to `step()` to set up the context for an invocation of the function. (Note: @@ -316,7 +289,7 @@ impl Connection { x_func: F) -> Result<()> where F: FnMut(&Context) -> Result, - T: ToResult + T: ToSql { self.db.borrow_mut().create_scalar_function(fn_name, n_arg, deterministic, x_func) } @@ -333,7 +306,7 @@ impl Connection { aggr: D) -> Result<()> where D: Aggregate, - T: ToResult + T: ToSql { self.db .borrow_mut() @@ -361,13 +334,13 @@ impl InnerConnection { x_func: F) -> Result<()> where F: FnMut(&Context) -> Result, - T: ToResult + T: ToSql { unsafe extern "C" fn call_boxed_closure(ctx: *mut sqlite3_context, argc: c_int, argv: *mut *mut sqlite3_value) where F: FnMut(&Context) -> Result, - T: ToResult + T: ToSql { let ctx = Context { ctx: ctx, @@ -375,20 +348,14 @@ impl InnerConnection { }; let boxed_f: *mut F = mem::transmute(ffi::sqlite3_user_data(ctx.ctx)); assert!(!boxed_f.is_null(), "Internal error - null function pointer"); - match (*boxed_f)(&ctx) { - Ok(r) => r.set_result(ctx.ctx), - Err(Error::SqliteFailure(err, s)) => { - ffi::sqlite3_result_error_code(ctx.ctx, err.extended_code); - if let Some(Ok(cstr)) = s.map(|s| str_to_cstring(&s)) { - ffi::sqlite3_result_error(ctx.ctx, cstr.as_ptr(), -1); - } - } - Err(err) => { - ffi::sqlite3_result_error_code(ctx.ctx, ffi::SQLITE_CONSTRAINT_FUNCTION); - if let Ok(cstr) = str_to_cstring(err.description()) { - ffi::sqlite3_result_error(ctx.ctx, cstr.as_ptr(), -1); - } - } + + let t = (*boxed_f)(&ctx); + let t = t.as_ref().map(|t| ToSql::to_sql(t)); + + match t { + Ok(Ok(ref value)) => set_result(ctx.ctx, value), + Ok(Err(err)) => report_error(ctx.ctx, &err), + Err(err) => report_error(ctx.ctx, err), } } @@ -419,7 +386,7 @@ impl InnerConnection { aggr: D) -> Result<()> where D: Aggregate, - T: ToResult + T: ToSql { unsafe fn aggregate_context(ctx: *mut sqlite3_context, bytes: usize) @@ -431,28 +398,11 @@ impl InnerConnection { Some(pac) } - unsafe fn report_aggregate_error(ctx: *mut sqlite3_context, err: Error) { - match err { - Error::SqliteFailure(err, s) => { - ffi::sqlite3_result_error_code(ctx, err.extended_code); - if let Some(Ok(cstr)) = s.map(|s| str_to_cstring(&s)) { - ffi::sqlite3_result_error(ctx, cstr.as_ptr(), -1); - } - } - _ => { - ffi::sqlite3_result_error_code(ctx, ffi::SQLITE_CONSTRAINT_FUNCTION); - if let Ok(cstr) = str_to_cstring(err.description()) { - ffi::sqlite3_result_error(ctx, cstr.as_ptr(), -1); - } - } - } - } - unsafe extern "C" fn call_boxed_step(ctx: *mut sqlite3_context, argc: c_int, argv: *mut *mut sqlite3_value) where D: Aggregate, - T: ToResult + T: ToSql { let boxed_aggr: *mut D = mem::transmute(ffi::sqlite3_user_data(ctx)); assert!(!boxed_aggr.is_null(), @@ -477,13 +427,13 @@ impl InnerConnection { match (*boxed_aggr).step(&mut ctx, &mut **pac) { Ok(_) => {} - Err(err) => report_aggregate_error(ctx.ctx, err), + Err(err) => report_error(ctx.ctx, &err), }; } unsafe extern "C" fn call_boxed_final(ctx: *mut sqlite3_context) where D: Aggregate, - T: ToResult + T: ToSql { let boxed_aggr: *mut D = mem::transmute(ffi::sqlite3_user_data(ctx)); assert!(!boxed_aggr.is_null(), @@ -503,10 +453,13 @@ impl InnerConnection { None => None, }; - match (*boxed_aggr).finalize(a) { - Ok(r) => r.set_result(ctx), - Err(err) => report_aggregate_error(ctx, err), - }; + let t = (*boxed_aggr).finalize(a); + let t = t.as_ref().map(|t| ToSql::to_sql(t)); + match t { + Ok(Ok(ref value)) => set_result(ctx, value), + Ok(Err(err)) => report_error(ctx, &err), + Err(err) => report_error(ctx, err), + } } let boxed_aggr: *mut D = Box::into_raw(Box::new(aggr)); From 2b830fde2df3719eaafd91ae009a4d02eaed59fd Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 May 2016 00:09:58 -0400 Subject: [PATCH 05/28] Remove pub re-export of FFI helpers from `functions`. --- src/functions.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/functions.rs b/src/functions.rs index a489475..290b855 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -57,10 +57,10 @@ use std::slice; use libc::{c_int, c_char, c_void}; use ffi; -pub use ffi::sqlite3_context; -pub use ffi::sqlite3_value; -pub use ffi::sqlite3_value_type; -pub use ffi::sqlite3_value_numeric_type; +use ffi::sqlite3_context; +use ffi::sqlite3_value; +use ffi::sqlite3_value_type; +use ffi::sqlite3_value_numeric_type; use types::{ToSql, ToSqlOutput, FromSql, ValueRef}; From 9a6e17b478be23adc0abe7d2e753b298c94f6951 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 May 2016 00:14:19 -0400 Subject: [PATCH 06/28] Add doc comments to ToSqlOutput. --- src/types/to_sql.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/types/to_sql.rs b/src/types/to_sql.rs index 1680f41..333bdbd 100644 --- a/src/types/to_sql.rs +++ b/src/types/to_sql.rs @@ -1,10 +1,15 @@ use super::{Null, Value, ValueRef}; use ::Result; +/// ToSqlOutput represents the possible output types for implementors of the ToSql trait. pub enum ToSqlOutput<'a> { + /// A borrowed SQLite-representable value. Borrowed(ValueRef<'a>), + + /// An owned SQLite-representable value. Owned(Value), + /// A BLOB of the given length that is filled with zeroes. #[cfg(feature = "blob")] ZeroBlob(i32), } From 60f7bddacc3c5d0064d6a5c371d774101dc11dea Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 May 2016 00:16:07 -0400 Subject: [PATCH 07/28] Add ToSql breaking change note to Changelog --- Changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.md b/Changelog.md index 233a58b..ed9b14d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,8 @@ * BREAKING CHANGE: The `FromSql` trait has been redesigned. It now requires a single, safe method instead of the previous definition which required implementing one or two unsafe methods. +* BREAKING CHANGE: The `ToSql` trait has been redesigned. It can now be implemented without + `unsafe`, and implementors can choose to return either borrowed or owned results. # Version 0.7.2 (2016-05-19) From f3693a993eefd72acc68b3ffa407025bcb8a51b2 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 May 2016 00:28:18 -0400 Subject: [PATCH 08/28] rustfmt --- src/functions.rs | 14 ++++++++++---- src/lib.rs | 18 +++++++++++++----- src/types/mod.rs | 3 ++- src/types/to_sql.rs | 8 ++++++-- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/functions.rs b/src/functions.rs index 290b855..033f9a5 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -106,7 +106,10 @@ fn set_result<'a>(ctx: *mut sqlite3_context, result: &ToSqlOutput<'a>) { } else if length == 0 { ffi::sqlite3_result_zeroblob(ctx, 0) } else { - ffi::sqlite3_result_blob(ctx, b.as_ptr() as *const c_void, length as c_int, ffi::SQLITE_TRANSIENT()); + ffi::sqlite3_result_blob(ctx, + b.as_ptr() as *const c_void, + length as c_int, + ffi::SQLITE_TRANSIENT()); } }, } @@ -139,7 +142,8 @@ impl<'a> ValueRef<'a> { ffi::SQLITE_FLOAT => ValueRef::Real(ffi::sqlite3_value_double(value)), ffi::SQLITE_TEXT => { let text = ffi::sqlite3_value_text(value); - assert!(!text.is_null(), "unexpected SQLITE_TEXT value type with NULL data"); + assert!(!text.is_null(), + "unexpected SQLITE_TEXT value type with NULL data"); let s = CStr::from_ptr(text as *const c_char); // sqlite3_value_text returns UTF8 data, so our unwrap here should be fine. @@ -148,10 +152,12 @@ impl<'a> ValueRef<'a> { } ffi::SQLITE_BLOB => { let blob = ffi::sqlite3_value_blob(value); - assert!(!blob.is_null(), "unexpected SQLITE_BLOB value type with NULL data"); + assert!(!blob.is_null(), + "unexpected SQLITE_BLOB value type with NULL data"); let len = ffi::sqlite3_value_bytes(value); - assert!(len >= 0, "unexpected negative return from sqlite3_value_bytes"); + assert!(len >= 0, + "unexpected negative return from sqlite3_value_bytes"); ValueRef::Blob(from_raw_parts(blob as *const u8, len as usize)) } diff --git a/src/lib.rs b/src/lib.rs index f8c6830..d08130d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -888,7 +888,8 @@ impl<'conn> Statement<'conn> { #[cfg(feature = "blob")] ToSqlOutput::ZeroBlob(len) => { - return self.conn.decode_result(unsafe { ffi::sqlite3_bind_zeroblob(ptr, col, len) }); + return self.conn + .decode_result(unsafe { ffi::sqlite3_bind_zeroblob(ptr, col, len) }); } }; self.conn.decode_result(match value { @@ -916,7 +917,11 @@ impl<'conn> Statement<'conn> { } else if length == 0 { ffi::sqlite3_bind_zeroblob(ptr, col, 0) } else { - ffi::sqlite3_bind_blob(ptr, col, b.as_ptr() as *const c_void, length as c_int, ffi::SQLITE_TRANSIENT()) + ffi::sqlite3_bind_blob(ptr, + col, + b.as_ptr() as *const c_void, + length as c_int, + ffi::SQLITE_TRANSIENT()) } }, }) @@ -1153,7 +1158,8 @@ impl<'a> ValueRef<'a> { ffi::SQLITE_FLOAT => ValueRef::Real(ffi::sqlite3_column_double(raw, col)), ffi::SQLITE_TEXT => { let text = ffi::sqlite3_column_text(raw, col); - assert!(!text.is_null(), "unexpected SQLITE_TEXT column type with NULL data"); + assert!(!text.is_null(), + "unexpected SQLITE_TEXT column type with NULL data"); let s = CStr::from_ptr(text as *const c_char); // sqlite3_column_text returns UTF8 data, so our unwrap here should be fine. @@ -1162,10 +1168,12 @@ impl<'a> ValueRef<'a> { } ffi::SQLITE_BLOB => { let blob = ffi::sqlite3_column_blob(raw, col); - assert!(!blob.is_null(), "unexpected SQLITE_BLOB column type with NULL data"); + assert!(!blob.is_null(), + "unexpected SQLITE_BLOB column type with NULL data"); let len = ffi::sqlite3_column_bytes(raw, col); - assert!(len >= 0, "unexpected negative return from sqlite3_column_bytes"); + assert!(len >= 0, + "unexpected negative return from sqlite3_column_bytes"); ValueRef::Blob(from_raw_parts(blob as *const u8, len as usize)) } diff --git a/src/types/mod.rs b/src/types/mod.rs index 92ca3fb..0a199c7 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -140,7 +140,8 @@ mod test { db.execute("INSERT INTO foo(i) VALUES (?)", &[&Value::Integer(10)]).unwrap(); - assert_eq!(10i64, db.query_row("SELECT i FROM foo", &[], |r| r.get(0)).unwrap()); + assert_eq!(10i64, + db.query_row("SELECT i FROM foo", &[], |r| r.get(0)).unwrap()); } #[test] diff --git a/src/types/to_sql.rs b/src/types/to_sql.rs index 333bdbd..157df04 100644 --- a/src/types/to_sql.rs +++ b/src/types/to_sql.rs @@ -14,7 +14,9 @@ pub enum ToSqlOutput<'a> { ZeroBlob(i32), } -impl<'a, T: ?Sized> From<&'a T> for ToSqlOutput<'a> where &'a T: Into> { +impl<'a, T: ?Sized> From<&'a T> for ToSqlOutput<'a> + where &'a T: Into> +{ fn from(t: &'a T) -> Self { ToSqlOutput::Borrowed(t.into()) } @@ -59,7 +61,9 @@ to_sql_self!(i32); to_sql_self!(i64); to_sql_self!(f64); -impl<'a, T: ?Sized> ToSql for &'a T where &'a T: Into> { +impl<'a, T: ?Sized> ToSql for &'a T + where &'a T: Into> +{ fn to_sql(&self) -> Result { Ok(ToSqlOutput::from((*self).into())) } From bafa85a1a010417dc3c3279163f49151fcf4e424 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Thu, 26 May 2016 00:30:01 -0400 Subject: [PATCH 09/28] Fix clippy warnings. --- src/types/to_sql.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/to_sql.rs b/src/types/to_sql.rs index 157df04..d6b3daf 100644 --- a/src/types/to_sql.rs +++ b/src/types/to_sql.rs @@ -1,7 +1,7 @@ use super::{Null, Value, ValueRef}; use ::Result; -/// ToSqlOutput represents the possible output types for implementors of the ToSql trait. +/// `ToSqlOutput` represents the possible output types for implementors of the `ToSql` trait. pub enum ToSqlOutput<'a> { /// A borrowed SQLite-representable value. Borrowed(ValueRef<'a>), From f817ec86bc6ced5a2b2fda791619bc8339c28dc3 Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 26 May 2016 21:16:09 +0200 Subject: [PATCH 10/28] Use new Rust 1.9 attribute: #[deprecated]. --- appveyor.yml | 2 +- src/error.rs | 1 + src/lib.rs | 7 +++++++ src/load_extension_guard.rs | 1 + src/transaction.rs | 2 ++ 5 files changed, 12 insertions(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index e15cb90..50054f6 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,5 +1,5 @@ environment: - TARGET: 1.8.0-x86_64-pc-windows-gnu + TARGET: 1.9.0-x86_64-pc-windows-gnu MSYS2_BITS: 64 install: - ps: Start-FileDownload "https://static.rust-lang.org/dist/rust-${env:TARGET}.exe" diff --git a/src/error.rs b/src/error.rs index 1d6ccd4..580468a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,6 +6,7 @@ use libc::c_int; use {ffi, errmsg_to_string}; /// Old name for `Error`. `SqliteError` is deprecated. +#[deprecated] pub type SqliteError = Error; /// Enum listing possible errors from rusqlite. diff --git a/src/lib.rs b/src/lib.rs index 8d84890..12331a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,6 +105,7 @@ mod raw_statement; const STATEMENT_CACHE_DEFAULT_CAPACITY: usize = 16; /// Old name for `Result`. `SqliteResult` is deprecated. +#[deprecated] pub type SqliteResult = Result; /// A typedef of the result returned by many methods. @@ -151,6 +152,7 @@ impl<'a> DatabaseName<'a> { } /// Old name for `Connection`. `SqliteConnection` is deprecated. +#[deprecated] pub type SqliteConnection = Connection; /// A connection to a SQLite database. @@ -367,6 +369,7 @@ impl Connection { /// /// This method should be considered deprecated. Use `query_row` instead, which now /// does exactly the same thing. + #[deprecated] pub fn query_row_safe(&self, sql: &str, params: &[&ToSql], f: F) -> Result where F: FnOnce(Row) -> T { @@ -507,6 +510,7 @@ struct InnerConnection { } /// Old name for `OpenFlags`. `SqliteOpenFlags` is deprecated. +#[deprecated] pub type SqliteOpenFlags = OpenFlags; bitflags! { @@ -678,6 +682,7 @@ impl Drop for InnerConnection { } /// Old name for `Statement`. `SqliteStatement` is deprecated. +#[deprecated] pub type SqliteStatement<'conn> = Statement<'conn>; /// A prepared statement. @@ -967,6 +972,7 @@ impl<'stmt, T, E, F> Iterator for AndThenRows<'stmt, F> } /// Old name for `Rows`. `SqliteRows` is deprecated. +#[deprecated] pub type SqliteRows<'stmt> = Rows<'stmt>; /// An handle for the resulting rows of a query. @@ -1031,6 +1037,7 @@ impl<'stmt> Drop for Rows<'stmt> { } /// Old name for `Row`. `SqliteRow` is deprecated. +#[deprecated] pub type SqliteRow<'a, 'stmt> = Row<'a, 'stmt>; /// A single result row of a query. diff --git a/src/load_extension_guard.rs b/src/load_extension_guard.rs index 7a02db4..c8f25b8 100644 --- a/src/load_extension_guard.rs +++ b/src/load_extension_guard.rs @@ -1,6 +1,7 @@ use {Result, Connection}; /// Old name for `LoadExtensionGuard`. `SqliteLoadExtensionGuard` is deprecated. +#[deprecated] pub type SqliteLoadExtensionGuard<'conn> = LoadExtensionGuard<'conn>; /// RAII guard temporarily enabling SQLite extensions to be loaded. diff --git a/src/transaction.rs b/src/transaction.rs index e678215..e66ccaa 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -2,6 +2,7 @@ use std::ops::Deref; use {Result, Connection}; /// Old name for `TransactionBehavior`. `SqliteTransactionBehavior` is deprecated. +#[deprecated] pub type SqliteTransactionBehavior = TransactionBehavior; /// Options for transaction behavior. See [BEGIN @@ -28,6 +29,7 @@ pub enum DropBehavior { } /// Old name for `Transaction`. `SqliteTransaction` is deprecated. +#[deprecated] pub type SqliteTransaction<'conn> = Transaction<'conn>; /// Represents a transaction on a database connection. From 6a4eacc927e062185c40150833d8090b35326c2c Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 28 May 2016 11:16:55 +0200 Subject: [PATCH 11/28] Simply use `cargo clippy` --- Cargo.toml | 1 - src/lib.rs | 2 -- 2 files changed, 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index df70e47..5085c32 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ 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 } serde_json = { version = "0.6", optional = true } diff --git a/src/lib.rs b/src/lib.rs index 8d84890..bee0f1f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,8 +50,6 @@ //! } //! } //! ``` -#![cfg_attr(feature="clippy", feature(plugin))] -#![cfg_attr(feature="clippy", plugin(clippy))] extern crate libc; extern crate libsqlite3_sys as ffi; From 7c0eba0475896f25d203f94ec22fc07073dbc7ed Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 29 May 2016 20:36:20 -0400 Subject: [PATCH 12/28] Add `since` and `note` for all deprecation tags. --- src/error.rs | 2 +- src/lib.rs | 14 +++++++------- src/load_extension_guard.rs | 2 +- src/transaction.rs | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/error.rs b/src/error.rs index 580468a..aadf688 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,7 +6,7 @@ use libc::c_int; use {ffi, errmsg_to_string}; /// Old name for `Error`. `SqliteError` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use Error instead")] pub type SqliteError = Error; /// Enum listing possible errors from rusqlite. diff --git a/src/lib.rs b/src/lib.rs index 12331a3..d550111 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,7 +105,7 @@ mod raw_statement; const STATEMENT_CACHE_DEFAULT_CAPACITY: usize = 16; /// Old name for `Result`. `SqliteResult` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use Result instead")] pub type SqliteResult = Result; /// A typedef of the result returned by many methods. @@ -152,7 +152,7 @@ impl<'a> DatabaseName<'a> { } /// Old name for `Connection`. `SqliteConnection` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use Connection instead")] pub type SqliteConnection = Connection; /// A connection to a SQLite database. @@ -369,7 +369,7 @@ impl Connection { /// /// This method should be considered deprecated. Use `query_row` instead, which now /// does exactly the same thing. - #[deprecated] + #[deprecated(since = "0.1.0", note = "Use query_row instead")] pub fn query_row_safe(&self, sql: &str, params: &[&ToSql], f: F) -> Result where F: FnOnce(Row) -> T { @@ -510,7 +510,7 @@ struct InnerConnection { } /// Old name for `OpenFlags`. `SqliteOpenFlags` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use OpenFlags instead")] pub type SqliteOpenFlags = OpenFlags; bitflags! { @@ -682,7 +682,7 @@ impl Drop for InnerConnection { } /// Old name for `Statement`. `SqliteStatement` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use Statement instead")] pub type SqliteStatement<'conn> = Statement<'conn>; /// A prepared statement. @@ -972,7 +972,7 @@ impl<'stmt, T, E, F> Iterator for AndThenRows<'stmt, F> } /// Old name for `Rows`. `SqliteRows` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use Rows instead")] pub type SqliteRows<'stmt> = Rows<'stmt>; /// An handle for the resulting rows of a query. @@ -1037,7 +1037,7 @@ impl<'stmt> Drop for Rows<'stmt> { } /// Old name for `Row`. `SqliteRow` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use Row instead")] pub type SqliteRow<'a, 'stmt> = Row<'a, 'stmt>; /// A single result row of a query. diff --git a/src/load_extension_guard.rs b/src/load_extension_guard.rs index c8f25b8..92db69b 100644 --- a/src/load_extension_guard.rs +++ b/src/load_extension_guard.rs @@ -1,7 +1,7 @@ use {Result, Connection}; /// Old name for `LoadExtensionGuard`. `SqliteLoadExtensionGuard` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use LoadExtensionGuard instead")] pub type SqliteLoadExtensionGuard<'conn> = LoadExtensionGuard<'conn>; /// RAII guard temporarily enabling SQLite extensions to be loaded. diff --git a/src/transaction.rs b/src/transaction.rs index e66ccaa..347f5cb 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -2,7 +2,7 @@ use std::ops::Deref; use {Result, Connection}; /// Old name for `TransactionBehavior`. `SqliteTransactionBehavior` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use TransactionBehavior instead")] pub type SqliteTransactionBehavior = TransactionBehavior; /// Options for transaction behavior. See [BEGIN @@ -29,7 +29,7 @@ pub enum DropBehavior { } /// Old name for `Transaction`. `SqliteTransaction` is deprecated. -#[deprecated] +#[deprecated(since = "0.6.0", note = "Use Transaction instead")] pub type SqliteTransaction<'conn> = Transaction<'conn>; /// Represents a transaction on a database connection. From b1b438158d0901f3ac0e744e748312595c194fcc Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 29 May 2016 20:38:46 -0400 Subject: [PATCH 13/28] Remove workaround for Rust compiler bug that was fixed in 1.9. --- src/lib.rs | 5 +---- src/named_params.rs | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bee0f1f..7910941 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -881,10 +881,7 @@ impl<'conn> Statement<'conn> { for (i, p) in params.iter().enumerate() { try!(unsafe { self.conn.decode_result( - // This should be - // `p.bind_parameter(self.stmt.ptr(), (i + 1) as c_int)` - // but that doesn't compile until Rust 1.9 due to a compiler bug. - ToSql::bind_parameter(*p, self.stmt.ptr(), (i + 1) as c_int) + p.bind_parameter(self.stmt.ptr(), (i + 1) as c_int) ) }); } diff --git a/src/named_params.rs b/src/named_params.rs index d864736..5486074 100644 --- a/src/named_params.rs +++ b/src/named_params.rs @@ -205,10 +205,7 @@ impl<'conn> Statement<'conn> { for &(name, value) in params { if let Some(i) = try!(self.parameter_index(name)) { try!(self.conn.decode_result(unsafe { - // This should be - // `value.bind_parameter(self.stmt.ptr(), i)` - // but that doesn't compile until Rust 1.9 due to a compiler bug. - ToSql::bind_parameter(value, self.stmt.ptr(), i) + value.bind_parameter(self.stmt.ptr(), i) })); } else { return Err(Error::InvalidParameterName(name.into())); From 1de7f4ae069cbfe57e49228fc7db773a7deff686 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 29 May 2016 20:39:27 -0400 Subject: [PATCH 14/28] Add deprecation note to Changelog. --- Changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.md b/Changelog.md index 233a58b..265ddd3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,8 @@ * BREAKING CHANGE: The `FromSql` trait has been redesigned. It now requires a single, safe method instead of the previous definition which required implementing one or two unsafe methods. +* Added `#[deprecated(since = "...", note = "...")]` flags (new in Rust 1.9 for libraries) to + all deprecated APIs. # Version 0.7.2 (2016-05-19) From ad0b823560852e0939f78a6a4de4e3bda92b2a35 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 1 Jun 2016 20:52:22 -0400 Subject: [PATCH 15/28] Remove sanity check in `insert()` that could return `StatementFailedToInsertRow`. This was intended to detect an `UPDATE` query passed to `insert`, but incorrectly failed if inserts to different tables caused the same row ID to be returned from both. `UPDATE`s are no longer detectable. --- src/convenient.rs | 37 ++++++++++++++++++++----------------- src/error.rs | 9 +-------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/convenient.rs b/src/convenient.rs index 5f867b4..22817a5 100644 --- a/src/convenient.rs +++ b/src/convenient.rs @@ -4,18 +4,20 @@ use types::ToSql; impl<'conn> Statement<'conn> { /// Execute an INSERT and return the ROWID. /// + /// # Note + /// + /// This function is a convenience wrapper around `execute()` intended for queries that + /// insert a single item. It is possible to misuse this function in a way that it cannot + /// detect, such as by calling it on a statement which _updates_ a single item rather than + /// inserting one. Please don't do that. + /// /// # Failure + /// /// Will return `Err` if no row is inserted or many rows are inserted. pub fn insert(&mut self, params: &[&ToSql]) -> Result { - // Some non-insertion queries could still return 1 change (an UPDATE, for example), so - // to guard against that we can check that the connection's last_insert_rowid() changes - // after we execute the statement. - let prev_rowid = self.conn.last_insert_rowid(); let changes = try!(self.execute(params)); - let new_rowid = self.conn.last_insert_rowid(); match changes { - 1 if prev_rowid != new_rowid => Ok(new_rowid), - 1 if prev_rowid == new_rowid => Err(Error::StatementFailedToInsertRow), + 1 => Ok(self.conn.last_insert_rowid()), _ => Err(Error::StatementChangedRows(changes)), } } @@ -57,18 +59,19 @@ mod test { } #[test] - fn test_insert_failures() { + fn test_insert_different_tables() { + // Test for https://github.com/jgallagher/rusqlite/issues/171 let db = Connection::open_in_memory().unwrap(); - db.execute_batch("CREATE TABLE foo(x INTEGER UNIQUE)").unwrap(); - let mut insert = db.prepare("INSERT INTO foo (x) VALUES (?)").unwrap(); - let mut update = db.prepare("UPDATE foo SET x = ?").unwrap(); + db.execute_batch(r" + CREATE TABLE foo(x INTEGER); + CREATE TABLE bar(x INTEGER); + ") + .unwrap(); - assert_eq!(insert.insert(&[&1i32]).unwrap(), 1); - - match update.insert(&[&2i32]) { - Err(Error::StatementFailedToInsertRow) => (), - r => panic!("Unexpected result {:?}", r), - } + assert_eq!(db.prepare("INSERT INTO foo VALUES (10)").unwrap().insert(&[]).unwrap(), + 1); + assert_eq!(db.prepare("INSERT INTO bar VALUES (10)").unwrap().insert(&[]).unwrap(), + 1); } #[test] diff --git a/src/error.rs b/src/error.rs index 1d6ccd4..f5d37aa 100644 --- a/src/error.rs +++ b/src/error.rs @@ -55,10 +55,6 @@ pub enum Error { /// Error when a query that was expected to insert one row did not insert any or insert many. StatementChangedRows(c_int), - /// Error when a query that was expected to insert a row did not change the connection's - /// last_insert_rowid(). - StatementFailedToInsertRow, - /// Error returned by `functions::Context::get` when the function argument cannot be converted /// to the requested type. #[cfg(feature = "functions")] @@ -105,7 +101,6 @@ impl fmt::Display for Error { Error::InvalidColumnName(ref name) => write!(f, "Invalid column name: {}", name), Error::InvalidColumnType => write!(f, "Invalid column type"), Error::StatementChangedRows(i) => write!(f, "Query changed {} rows", i), - Error::StatementFailedToInsertRow => write!(f, "Statement failed to insert new row"), #[cfg(feature = "functions")] Error::InvalidFunctionParameterType => write!(f, "Invalid function parameter type"), @@ -136,7 +131,6 @@ impl error::Error for Error { Error::InvalidColumnName(_) => "invalid column name", Error::InvalidColumnType => "invalid column type", Error::StatementChangedRows(_) => "query inserted zero or more than one row", - Error::StatementFailedToInsertRow => "statement failed to insert new row", #[cfg(feature = "functions")] Error::InvalidFunctionParameterType => "invalid function parameter type", @@ -161,8 +155,7 @@ impl error::Error for Error { Error::InvalidColumnName(_) | Error::InvalidColumnType | Error::InvalidPath(_) | - Error::StatementChangedRows(_) | - Error::StatementFailedToInsertRow => None, + Error::StatementChangedRows(_) => None, #[cfg(feature = "functions")] Error::InvalidFunctionParameterType => None, From b235b89555bd59820db3eb9e2b30370f8593acac Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Wed, 1 Jun 2016 21:06:56 -0400 Subject: [PATCH 16/28] Bump to version 0.7.3. --- Cargo.toml | 2 +- Changelog.md | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index df70e47..01134ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rusqlite" -version = "0.7.2" +version = "0.7.3" authors = ["John Gallagher "] description = "Ergonomic wrapper for SQLite" repository = "https://github.com/jgallagher/rusqlite" diff --git a/Changelog.md b/Changelog.md index 358d93f..b7c2d76 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,9 @@ +# Version 0.7.3 (2016-06-01) + +* Fixes an incorrect failure from the `insert()` convenience function when back-to-back inserts to + different tables both returned the same row ID + ([#171](https://github.com/jgallagher/rusqlite/issues/171)). + # Version 0.7.2 (2016-05-19) * BREAKING CHANGE: `Rows` no longer implements `Iterator`. It still has a `next()` method, but From 95050f10a848cd6861047715f65cf3c421024043 Mon Sep 17 00:00:00 2001 From: gwenn Date: Mon, 13 Jun 2016 20:22:21 +0200 Subject: [PATCH 17/28] Add test with empty blob (issue #174). --- src/types/mod.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/types/mod.rs b/src/types/mod.rs index 758eb5d..61deaf6 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -129,6 +129,17 @@ mod test { assert_eq!(v, v1234); } + #[test] + fn test_empty_blob() { + let db = checked_memory_handle(); + + let empty = vec![]; + db.execute("INSERT INTO foo(b) VALUES (?)", &[&empty]).unwrap(); + + let v: Vec = db.query_row("SELECT b FROM foo", &[], |r| r.get(0)).unwrap(); + assert_eq!(v, empty); + } + #[test] fn test_str() { let db = checked_memory_handle(); From 9f05147660096c9d68a4c60cfc88bba6861dd015 Mon Sep 17 00:00:00 2001 From: gwenn Date: Mon, 13 Jun 2016 20:32:39 +0200 Subject: [PATCH 18/28] Fix issue jgallagher/rusqlite#174 --- src/lib.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c571704..50cf7e9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1123,12 +1123,17 @@ impl<'a> ValueRef<'a> { } ffi::SQLITE_BLOB => { let blob = ffi::sqlite3_column_blob(raw, col); - assert!(!blob.is_null(), "unexpected SQLITE_BLOB column type with NULL data"); let len = ffi::sqlite3_column_bytes(raw, col); assert!(len >= 0, "unexpected negative return from sqlite3_column_bytes"); + if len > 0 { + assert!(!blob.is_null(), "unexpected SQLITE_BLOB column type with NULL data"); + ValueRef::Blob(from_raw_parts(blob as *const u8, len as usize)) + } else { + // The return value from sqlite3_column_blob() for a zero-length BLOB is a NULL pointer. + ValueRef::Blob(&[]) + } - ValueRef::Blob(from_raw_parts(blob as *const u8, len as usize)) } _ => unreachable!("sqlite3_column_type returned invalid value") } From a7d27098b73e46fecd48f4a7a06dd4337a9c5905 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 2 Jul 2016 10:22:47 +0200 Subject: [PATCH 19/28] Fix issue #177 --- README.md | 2 +- src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2f8d23c..59d5204 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ fn main() { data: None }; conn.execute("INSERT INTO person (name, time_created, data) - VALUES ($1, $2, $3)", + VALUES (?1, ?2, ?3)", &[&me.name, &me.time_created, &me.data]).unwrap(); let mut stmt = conn.prepare("SELECT id, name, time_created, data FROM person").unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 50cf7e9..d429308 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,7 +32,7 @@ //! data: None //! }; //! conn.execute("INSERT INTO person (name, time_created, data) -//! VALUES ($1, $2, $3)", +//! VALUES (?1, ?2, ?3)", //! &[&me.name, &me.time_created, &me.data]).unwrap(); //! //! let mut stmt = conn.prepare("SELECT id, name, time_created, data FROM person").unwrap(); From ece7c041e85a215cfb9c7d15884d11dbf036c087 Mon Sep 17 00:00:00 2001 From: Patrick Fernie Date: Fri, 7 Oct 2016 12:42:27 -0400 Subject: [PATCH 20/28] change query_row* fns to take Row by reference instead of moving --- src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 50cf7e9..3e3199e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -303,12 +303,12 @@ impl Connection { /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the /// underlying SQLite call fails. pub fn query_row(&self, sql: &str, params: &[&ToSql], f: F) -> Result - where F: FnOnce(Row) -> T + where F: FnOnce(&Row) -> T { let mut stmt = try!(self.prepare(sql)); let mut rows = try!(stmt.query(params)); - rows.get_expected_row().map(f) + rows.get_expected_row().map(|r| f(&r)) } /// Convenience method to execute a query that is expected to return a single row, @@ -339,13 +339,13 @@ impl Connection { params: &[&ToSql], f: F) -> result::Result - where F: FnOnce(Row) -> result::Result, + where F: FnOnce(&Row) -> result::Result, E: convert::From { let mut stmt = try!(self.prepare(sql)); let mut rows = try!(stmt.query(params)); - rows.get_expected_row().map_err(E::from).and_then(f) + rows.get_expected_row().map_err(E::from).and_then(|r| f(&r)) } /// Convenience method to execute a query that is expected to return a single row. @@ -369,7 +369,7 @@ impl Connection { /// does exactly the same thing. #[deprecated(since = "0.1.0", note = "Use query_row instead")] pub fn query_row_safe(&self, sql: &str, params: &[&ToSql], f: F) -> Result - where F: FnOnce(Row) -> T + where F: FnOnce(&Row) -> T { self.query_row(sql, params, f) } From d1fd4a371d91d9bf03eb4d7a353663242ce65cf7 Mon Sep 17 00:00:00 2001 From: gwenn Date: Fri, 7 Oct 2016 20:14:09 +0200 Subject: [PATCH 21/28] Add link to crates.io --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2f8d23c..a1a0126 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Rusqlite [![Travis Build Status](https://api.travis-ci.org/jgallagher/rusqlite.svg?branch=master)](https://travis-ci.org/jgallagher/rusqlite) -[![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/github/jgallagher/rusqlite?branch=master&svg=true)](https://ci.appveyor.com/project/jgallagher/rusqlite) +[![AppVeyor Build Status](https://ci.appveyor.com/api/projects/status/github/jgallagher/rusqlite?branch=master&svg=true)](https://ci.appveyor.com/project/jgallagher/rusqlite) [![Latest Version](https://img.shields.io/crates/v/rusqlite.svg)](https://crates.io/crates/rusqlite) Rusqlite is an ergonomic wrapper for using SQLite from Rust. It attempts to expose an interface similar to [rust-postgres](https://github.com/sfackler/rust-postgres). View the full From ffe605150afecf0bbdffb836b62a1ba68257b091 Mon Sep 17 00:00:00 2001 From: gwenn Date: Fri, 4 Nov 2016 20:47:28 +0100 Subject: [PATCH 22/28] Ensure cache is flushed when closing the connection Fix #186 --- Cargo.toml | 2 +- src/cache.rs | 17 +++++++++++++++++ src/lib.rs | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index c5a156b..df88455 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,7 @@ trace = [] [dependencies] time = "~0.1.0" bitflags = "0.7" -lru-cache = "0.0.7" +lru-cache = "0.1.0" libc = "~0.2" chrono = { version = "~0.2", optional = true } serde_json = { version = "0.6", optional = true } diff --git a/src/cache.rs b/src/cache.rs index d246e8e..cba4e75 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -44,6 +44,10 @@ impl Connection { pub fn set_prepared_statement_cache_capacity(&self, capacity: usize) { self.cache.set_capacity(capacity) } + + pub fn flush_prepared_statement_cache(&self) { + self.cache.flush() + } } /// Prepared statements LRU cache. @@ -133,6 +137,11 @@ impl StatementCache { let sql = String::from_utf8_lossy(stmt.sql().to_bytes()).to_string(); cache.insert(sql, stmt); } + + fn flush(&self) { + let mut cache = self.0.borrow_mut(); + cache.clear() + } } #[cfg(test)] @@ -268,4 +277,12 @@ mod test { .unwrap()); } } + + #[test] + fn test_connection_close() { + let conn = Connection::open_in_memory().unwrap(); + conn.prepare_cached("SELECT * FROM sqlite_master;").unwrap(); + + conn.close().expect("connection not closed"); + } } diff --git a/src/lib.rs b/src/lib.rs index 50cf7e9..4a9d53b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -405,6 +405,7 @@ impl Connection { /// /// Will return `Err` if the underlying SQLite call fails. pub fn close(self) -> Result<()> { + self.flush_prepared_statement_cache(); let mut db = self.db.borrow_mut(); db.close() } From 4f5abc705a147c3bb3e2baeecbfabdf14f0f3033 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 30 Dec 2016 23:25:21 -0500 Subject: [PATCH 23/28] Remove workaround for issue fixed in Rust 1.9. --- src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 780cb7f..76c9729 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -878,11 +878,7 @@ impl<'conn> Statement<'conn> { } fn bind_parameter(&self, param: &ToSql, col: c_int) -> Result<()> { - // This should be - // let value = try!(param.to_sql()); - // but that hits a bug in the Rust compiler around re-exported - // trait visibility. It's fixed in 1.9. - let value = try!(ToSql::to_sql(param)); + let value = try!(param.to_sql()); let ptr = unsafe { self.stmt.ptr() }; let value = match value { From f17fc14a5968432255c60f700dfe25e397b3157d Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 30 Dec 2016 23:48:04 -0500 Subject: [PATCH 24/28] Update query_row_named so its closure also takes a &Row instead of a Row --- src/named_params.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/named_params.rs b/src/named_params.rs index 7529215..d4a5f47 100644 --- a/src/named_params.rs +++ b/src/named_params.rs @@ -38,12 +38,12 @@ impl Connection { /// Will return `Err` if `sql` cannot be converted to a C-compatible string or if the /// underlying SQLite call fails. pub fn query_row_named(&self, sql: &str, params: &[(&str, &ToSql)], f: F) -> Result - where F: FnOnce(Row) -> T + where F: FnOnce(&Row) -> T { let mut stmt = try!(self.prepare(sql)); let mut rows = try!(stmt.query_named(params)); - rows.get_expected_row().map(f) + rows.get_expected_row().map(|r| f(&r)) } } From c95c3acc8e2e765a42c5e64b2755f394cce6bbc0 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 30 Dec 2016 23:48:24 -0500 Subject: [PATCH 25/28] Add breaking change note about Row -> &Row in closures to Changelog --- Changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Changelog.md b/Changelog.md index ea4b6f8..9505465 100644 --- a/Changelog.md +++ b/Changelog.md @@ -5,6 +5,10 @@ methods. * BREAKING CHANGE: The `ToSql` trait has been redesigned. It can now be implemented without `unsafe`, and implementors can choose to return either borrowed or owned results. +* BREAKING CHANGE: The closure passed to `query_row`, `query_row_and_then`, `query_row_safe`, + and `query_row_named` now expects a `&Row` instead of a `Row`. The vast majority of calls + to these functions will probably not need to change; see + https://github.com/jgallagher/rusqlite/pull/184. * Added `#[deprecated(since = "...", note = "...")]` flags (new in Rust 1.9 for libraries) to all deprecated APIs. From 674419e080d09dfdc5d1fbc32b2c3a1870b640e3 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Fri, 30 Dec 2016 23:58:12 -0500 Subject: [PATCH 26/28] Add bugfix note to Changelog --- Changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changelog.md b/Changelog.md index 9505465..7e1d231 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,8 @@ https://github.com/jgallagher/rusqlite/pull/184. * Added `#[deprecated(since = "...", note = "...")]` flags (new in Rust 1.9 for libraries) to all deprecated APIs. +* Fixed a bug where using cached prepared statements resulted in attempting to close a connection + failing with `DatabaseBusy`; see https://github.com/jgallagher/rusqlite/issues/186. # Version 0.7.3 (2016-06-01) From bc71c583872b564c1604abb0fb07dfe58311009c Mon Sep 17 00:00:00 2001 From: gwenn Date: Thu, 14 Jul 2016 06:27:46 +0200 Subject: [PATCH 27/28] Add Statement.query_row convenient method (#179) --- src/convenient.rs | 33 +++++++++++++++++++++++++++++++-- src/lib.rs | 4 +--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/convenient.rs b/src/convenient.rs index 22817a5..4740aa2 100644 --- a/src/convenient.rs +++ b/src/convenient.rs @@ -1,4 +1,4 @@ -use {Error, Result, Statement}; +use {Error, Result, Row, Statement}; use types::ToSql; impl<'conn> Statement<'conn> { @@ -34,11 +34,26 @@ impl<'conn> Statement<'conn> { }; Ok(exists) } + + /// Convenience method to execute a query that is expected to return a single row. + /// + /// If the query returns more than one row, all rows except the first are ignored. + /// + /// # Failure + /// + /// Will return `Err` if the underlying SQLite call fails. + pub fn query_row(&mut self, params: &[&ToSql], f: F) -> Result + where F: FnOnce(&Row) -> T + { + let mut rows = try!(self.query(params)); + + rows.get_expected_row().map(|r| f(&r)) + } } #[cfg(test)] mod test { - use {Connection, Error}; + use {Connection, Error, Result}; #[test] fn test_insert() { @@ -88,4 +103,18 @@ mod test { assert!(stmt.exists(&[&2i32]).unwrap()); assert!(!stmt.exists(&[&0i32]).unwrap()); } + + #[test] + fn test_query_row() { + let db = Connection::open_in_memory().unwrap(); + let sql = "BEGIN; + CREATE TABLE foo(x INTEGER, y INTEGER); + INSERT INTO foo VALUES(1, 3); + INSERT INTO foo VALUES(2, 4); + END;"; + db.execute_batch(sql).unwrap(); + let mut stmt = db.prepare("SELECT y FROM foo WHERE x = ?").unwrap(); + let y: Result = stmt.query_row(&[&1i32], |r| r.get(0)); + assert_eq!(3i64, y.unwrap()); + } } diff --git a/src/lib.rs b/src/lib.rs index ed0f300..ab7a08c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -306,9 +306,7 @@ impl Connection { where F: FnOnce(&Row) -> T { let mut stmt = try!(self.prepare(sql)); - let mut rows = try!(stmt.query(params)); - - rows.get_expected_row().map(|r| f(&r)) + stmt.query_row(params, f) } /// Convenience method to execute a query that is expected to return a single row, From ff311ec29ab26072ebeb29045beaee6e2d3365b1 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sat, 31 Dec 2016 00:04:23 -0500 Subject: [PATCH 28/28] Add Statement::query_row note to Changelog. --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 7e1d231..10a0f3f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,7 @@ https://github.com/jgallagher/rusqlite/pull/184. * Added `#[deprecated(since = "...", note = "...")]` flags (new in Rust 1.9 for libraries) to all deprecated APIs. +* Added `query_row` convenience function to `Statement`. * Fixed a bug where using cached prepared statements resulted in attempting to close a connection failing with `DatabaseBusy`; see https://github.com/jgallagher/rusqlite/issues/186.