From dd886578d24a2e231cd7546ccad731762563daba Mon Sep 17 00:00:00 2001 From: gwenn Date: Fri, 26 Jun 2020 19:35:14 +0200 Subject: [PATCH 1/2] Implement our own sqlite3_exec Should fix issue related to unlock notify: #767 Caveat: many CString allocated. --- src/inner_connection.rs | 28 +++++++++++----------------- src/lib.rs | 16 +++++++++++++--- src/pragma.rs | 3 +-- src/raw_statement.rs | 8 ++++++-- src/statement.rs | 4 ++-- src/transaction.rs | 16 +++++++--------- 6 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/inner_connection.rs b/src/inner_connection.rs index f967584..dd786fe 100644 --- a/src/inner_connection.rs +++ b/src/inner_connection.rs @@ -171,21 +171,6 @@ impl InnerConnection { } } - pub fn execute_batch(&mut self, sql: &str) -> Result<()> { - // use CString instead of SmallCString because it's probably big. - let c_sql = std::ffi::CString::new(sql)?; - unsafe { - let r = ffi::sqlite3_exec( - self.db(), - c_sql.as_ptr(), - None, - ptr::null_mut(), - ptr::null_mut(), - ); - self.decode_result(r) - } - } - #[cfg(feature = "load_extension")] pub fn enable_load_extension(&mut self, onoff: c_int) -> Result<()> { let r = unsafe { ffi::sqlite3_enable_load_extension(self.db, onoff) }; @@ -262,8 +247,17 @@ impl InnerConnection { // comment) then *ppStmt is set to NULL. let c_stmt: *mut ffi::sqlite3_stmt = c_stmt; let c_tail: *const c_char = c_tail; - // TODO ignore spaces, comments, ... at the end - let tail = !c_tail.is_null() && unsafe { c_tail != c_sql.offset(len as isize) }; + let tail = if c_tail.is_null() { + 0 + } else { + // TODO nightly feature ptr_offset_from #41079 + let n = (c_tail as isize) - (c_sql as isize); + if n <= 0 || n >= len as isize { + 0 + } else { + n as usize + } + }; Ok(Statement::new(conn, unsafe { RawStatement::new(c_stmt, tail) })) diff --git a/src/lib.rs b/src/lib.rs index a774be1..f768a65 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -435,8 +435,6 @@ impl Connection { /// Convenience method to run multiple SQL statements (that cannot take any /// parameters). /// - /// Uses [sqlite3_exec](http://www.sqlite.org/c3ref/exec.html) under the hood. - /// /// ## Example /// /// ```rust,no_run @@ -456,7 +454,19 @@ impl Connection { /// Will return `Err` if `sql` cannot be converted to a C-compatible string /// or if the underlying SQLite call fails. pub fn execute_batch(&self, sql: &str) -> Result<()> { - self.db.borrow_mut().execute_batch(sql) + let mut sql = sql; + while !sql.is_empty() { + let stmt = self.prepare(sql)?; + if !stmt.stmt.is_null() && stmt.step()? { + return Err(Error::ExecuteReturnedResults); + } + let tail = stmt.stmt.tail(); + if tail == 0 || tail >= sql.len() { + break; + } + sql = &sql[tail..]; + } + Ok(()) } /// Convenience method to prepare and execute a single SQL statement. diff --git a/src/pragma.rs b/src/pragma.rs index b1dc77f..b115f8f 100644 --- a/src/pragma.rs +++ b/src/pragma.rs @@ -256,8 +256,7 @@ impl Connection { // The two syntaxes yield identical results. sql.push_equal_sign(); sql.push_value(pragma_value)?; - self.execute(&sql, NO_PARAMS)?; - Ok(()) + self.execute_batch(&sql) } /// Set a new value to `pragma_name` and return the updated value. diff --git a/src/raw_statement.rs b/src/raw_statement.rs index e16aa96..c02dcd9 100644 --- a/src/raw_statement.rs +++ b/src/raw_statement.rs @@ -12,7 +12,7 @@ use std::sync::Arc; #[derive(Debug)] pub struct RawStatement { ptr: *mut ffi::sqlite3_stmt, - tail: bool, + tail: usize, // Cached indices of named parameters, computed on the fly. cache: crate::util::ParamIndexCache, // Cached SQL (trimmed) that we use as the key when we're in the statement @@ -29,7 +29,7 @@ pub struct RawStatement { } impl RawStatement { - pub unsafe fn new(stmt: *mut ffi::sqlite3_stmt, tail: bool) -> RawStatement { + pub unsafe fn new(stmt: *mut ffi::sqlite3_stmt, tail: usize) -> RawStatement { RawStatement { ptr: stmt, tail, @@ -170,6 +170,10 @@ impl RawStatement { #[cfg(feature = "extra_check")] pub fn has_tail(&self) -> bool { + self.tail != 0 + } + + pub fn tail(&self) -> usize { self.tail } } diff --git a/src/statement.rs b/src/statement.rs index 9133dda..7657095 100644 --- a/src/statement.rs +++ b/src/statement.rs @@ -622,7 +622,7 @@ impl Statement<'_> { } fn finalize_(&mut self) -> Result<()> { - let mut stmt = unsafe { RawStatement::new(ptr::null_mut(), false) }; + let mut stmt = unsafe { RawStatement::new(ptr::null_mut(), 0) }; mem::swap(&mut stmt, &mut self.stmt); self.conn.decode_result(stmt.finalize()) } @@ -707,7 +707,7 @@ impl Statement<'_> { /// connection has closed is illegal, but `RawStatement` does not enforce /// this, as it loses our protective `'conn` lifetime bound. pub(crate) unsafe fn into_raw(mut self) -> RawStatement { - let mut stmt = RawStatement::new(ptr::null_mut(), false); + let mut stmt = RawStatement::new(ptr::null_mut(), 0); mem::swap(&mut stmt, &mut self.stmt); stmt } diff --git a/src/transaction.rs b/src/transaction.rs index 3e32f44..5e649b7 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -1,4 +1,4 @@ -use crate::{Connection, Result, NO_PARAMS}; +use crate::{Connection, Result}; use std::ops::Deref; /// Options for transaction behavior. See [BEGIN @@ -120,7 +120,7 @@ impl Transaction<'_> { TransactionBehavior::Immediate => "BEGIN IMMEDIATE", TransactionBehavior::Exclusive => "BEGIN EXCLUSIVE", }; - conn.execute(query, NO_PARAMS).map(move |_| Transaction { + conn.execute_batch(query).map(move |_| Transaction { conn, drop_behavior: DropBehavior::Rollback, }) @@ -180,7 +180,7 @@ impl Transaction<'_> { } fn commit_(&mut self) -> Result<()> { - self.conn.execute("COMMIT", NO_PARAMS)?; + self.conn.execute_batch("COMMIT")?; Ok(()) } @@ -190,7 +190,7 @@ impl Transaction<'_> { } fn rollback_(&mut self) -> Result<()> { - self.conn.execute("ROLLBACK", NO_PARAMS)?; + self.conn.execute_batch("ROLLBACK")?; Ok(()) } @@ -238,7 +238,7 @@ impl Savepoint<'_> { name: T, ) -> Result> { let name = name.into(); - conn.execute(&format!("SAVEPOINT {}", name), NO_PARAMS) + conn.execute_batch(&format!("SAVEPOINT {}", name)) .map(|_| Savepoint { conn, name, @@ -291,8 +291,7 @@ impl Savepoint<'_> { } fn commit_(&mut self) -> Result<()> { - self.conn - .execute(&format!("RELEASE {}", self.name), NO_PARAMS)?; + self.conn.execute_batch(&format!("RELEASE {}", self.name))?; self.committed = true; Ok(()) } @@ -305,8 +304,7 @@ impl Savepoint<'_> { /// rolled back, and can be rolled back again or committed. pub fn rollback(&mut self) -> Result<()> { self.conn - .execute(&format!("ROLLBACK TO {}", self.name), NO_PARAMS)?; - Ok(()) + .execute_batch(&format!("ROLLBACK TO {}", self.name)) } /// Consumes the savepoint, committing or rolling back according to the From 3cea0c7af53e83d8aa9e0976a2cdc15cc6f1486d Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 27 Jun 2020 09:54:33 +0200 Subject: [PATCH 2/2] Benchmark execute vs execute_batch --- Cargo.toml | 4 ++++ benches/exec.rs | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 benches/exec.rs diff --git a/Cargo.toml b/Cargo.toml index 5b94e22..76eef15 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -135,6 +135,10 @@ name = "vtab" name = "cache" harness = false +[[bench]] +name = "exec" +harness = false + [package.metadata.docs.rs] features = [ "backup", "blob", "chrono", "collation", "functions", "limits", "load_extension", "serde_json", "trace", "url", "vtab", "window", "modern_sqlite", "column_decltype" ] all-features = false diff --git a/benches/exec.rs b/benches/exec.rs new file mode 100644 index 0000000..360a98b --- /dev/null +++ b/benches/exec.rs @@ -0,0 +1,17 @@ +use bencher::{benchmark_group, benchmark_main, Bencher}; +use rusqlite::{Connection, NO_PARAMS}; + +fn bench_execute(b: &mut Bencher) { + let db = Connection::open_in_memory().unwrap(); + let sql = "PRAGMA user_version=1"; + b.iter(|| db.execute(sql, NO_PARAMS).unwrap()); +} + +fn bench_execute_batch(b: &mut Bencher) { + let db = Connection::open_in_memory().unwrap(); + let sql = "PRAGMA user_version=1"; + b.iter(|| db.execute_batch(sql).unwrap()); +} + +benchmark_group!(exec_benches, bench_execute, bench_execute_batch); +benchmark_main!(exec_benches);