From d75a3aa4a13102d2b3c0814ec70d1d549090dbed Mon Sep 17 00:00:00 2001 From: Joao Henrique Machado Silva Date: Fri, 8 May 2026 14:42:45 +0200 Subject: [PATCH] feat(engine): prepared-statement plan cache + parameter binding (SQLR-23) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Connection::prepare_cached` (default 16-entry per-connection LRU) + `Statement::query_with_params` / `Statement::execute_with_params` substitute `?` placeholders into a cached AST at execute time, skipping the per-call sqlparser walk. `Value::Vector(Vec)` is a first-class bind type — a bound vector substitutes into the same in-band bracket- array shape an inline `[…]` literal would, so the HNSW probe optimizer still recognizes it on prepared queries. Bench harness flipped from per-call SQL formatting (`inline_params`) to the bound + cached path; every workload's `WorkloadId.version` bumped `v1 → v2` in lockstep so the methodology change is captured explicitly. Old v1 envelopes stay readable; the comparison script flags cross- version pairs. Smoke run confirms parser-bound wins on the workloads the plan predicted: W1 -52%, W6 -61%, W3 -44%, W11 -26%. Republishing the official pinned-host envelope is SQLR-25. Surfaced one pre-existing limitation: `try_hnsw_probe` is L2-only, so W10's `vec_distance_cosine` query has been silently brute-forcing the HNSW variant the entire time. SQLR-28 tracks widening the probe to cosine + dot. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 12 + Cargo.toml | 5 +- README.md | 2 +- benchmarks/src/data.rs | 5 + benchmarks/src/drivers/duckdb.rs | 28 +- benchmarks/src/drivers/sqlite.rs | 28 +- benchmarks/src/drivers/sqlrite.rs | 151 ++---- benchmarks/src/lib.rs | 19 +- benchmarks/src/workloads/aggregate.rs | 2 +- benchmarks/src/workloads/bulk_insert.rs | 2 +- benchmarks/src/workloads/fts.rs | 42 +- benchmarks/src/workloads/group_by.rs | 2 +- benchmarks/src/workloads/hybrid.rs | 59 ++- benchmarks/src/workloads/index_lookup.rs | 2 +- benchmarks/src/workloads/join.rs | 2 +- benchmarks/src/workloads/kv.rs | 2 +- benchmarks/src/workloads/mixed_oltp.rs | 2 +- benchmarks/src/workloads/range_scan.rs | 2 +- benchmarks/src/workloads/single_insert.rs | 2 +- benchmarks/src/workloads/vector.rs | 42 +- docs/architecture.md | 5 +- docs/benchmarks-plan.md | 2 +- docs/benchmarks.md | 11 +- docs/supported-sql.md | 28 +- src/connection.rs | 562 +++++++++++++++++++--- src/lib.rs | 4 +- src/sql/mod.rs | 13 +- src/sql/params.rs | 271 +++++++++++ 28 files changed, 1049 insertions(+), 258 deletions(-) create mode 100644 src/sql/params.rs diff --git a/Cargo.lock b/Cargo.lock index e900f0d..f532156 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4783,6 +4783,18 @@ checksum = "dbf5ea8d4d7c808e1af1cbabebca9a2abe603bcefc22294c5b95018d53200cb7" dependencies = [ "log", "recursive", + "sqlparser_derive", +] + +[[package]] +name = "sqlparser_derive" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6dd45d8fc1c79299bfbb7190e42ccbbdf6a5f52e4a6ad98d92357ea965bd289" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index ff31ff9..8a3f788 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -101,7 +101,10 @@ file-locks = ["dep:fs2"] [dependencies] # Always-on engine deps — pure-Rust and wasm-compatible. log = "0.4" -sqlparser = "0.61" +# SQLR-23: the `visitor` feature unlocks `visit_expressions_mut`, which +# we use for the prepared-statement `?` placeholder rewrite + bind +# substitution pass in `src/sql/params.rs`. +sqlparser = { version = "0.61", features = ["visitor"] } thiserror = "2.0" prettytable-rs = "0.10" # Phase 7e: JSON column type. `serde_json` powers both the validation diff --git a/README.md b/README.md index 729ce62..f96300a 100644 --- a/README.md +++ b/README.md @@ -275,7 +275,7 @@ The project is staged in phases, each independently shippable. A finished phase - [x] **4f — Transactions (`BEGIN` / `COMMIT` / `ROLLBACK`)**: `BEGIN` snapshots the in-memory tables (`Table::deep_clone`) and suppresses auto-save; every subsequent mutation stays in memory. `COMMIT` flushes accumulated changes in one `save_database` call (one WAL commit frame for the whole transaction). `ROLLBACK` restores the pre-BEGIN snapshot. Nested begins, orphan commits/rollbacks, and BEGIN on read-only DBs all return typed errors. Errors mid-transaction keep the transaction open so the caller can explicitly recover. **Phase 5 — Embedding surface: public API + language SDKs** -- [x] **5a — Public Rust API** *(partial)*: `Connection` / `Statement` / `Rows` / `Row` / `OwnedRow` / `FromValue` / `Value` at the crate root; structured row return from the executor; `examples/rust/quickstart.rs` runnable via `cargo run --example quickstart`. Parameter binding + cursor abstraction deferred to 5a.2. +- [x] **5a — Public Rust API**: `Connection` / `Statement` / `Rows` / `Row` / `OwnedRow` / `FromValue` / `Value` at the crate root; structured row return from the executor; `examples/rust/quickstart.rs` runnable via `cargo run --example quickstart`. **SQLR-23 — parameter binding + plan cache:** `Connection::prepare_cached` (default 16-entry LRU), `Statement::query_with_params(&[Value])`, `Statement::execute_with_params(&[Value])`. The cached AST skips re-running sqlparser per execute; `?` placeholders bind via positional `&[Value]`. `Value::Vector(Vec)` is a first-class bind type so HNSW-eligible KNN queries skip per-iter lexing of the 4 KB query vector — and the HNSW optimizer hook still recognizes the bound vector. Cursor abstraction still deferred to 5a.2. - [x] **5b — C FFI shim**: new `sqlrite-ffi/` workspace crate ships `libsqlrite_c.{so,dylib,dll}` + a cbindgen-generated `sqlrite.h`. Opaque-pointer types, thread-local last-error, split `sqlrite_execute` (DDL/DML/transactions) vs `sqlrite_query`/`sqlrite_step` (SELECT iteration). Runnable `examples/c/hello.c` + `Makefile` (`cd examples/c && make run`). - [x] **5c — Python SDK**: new `sdk/python/` workspace crate via PyO3 (`abi3-py38`) + maturin. DB-API 2.0-inspired — `sqlrite.connect(path)` → `Cursor.execute` / `fetchall` / iteration, context-manager support (commit-on-clean-exit / rollback-on-exception), read-only connections, 16-test pytest suite. `examples/python/hello.py` runs after `maturin develop`. PyPI publish landed in Phase 6f as `sqlrite`. - [x] **5d — Node.js SDK**: new `sdk/nodejs/` workspace crate via napi-rs (N-API v9, Node 18+). Prebuilt `.node` binaries — no `node-gyp` install step. `better-sqlite3`-style sync API (`new Database(path)`, `stmt.all() / get() / iterate()` returning row objects), auto-generated TypeScript defs, 11 `node:test` integration tests. `examples/nodejs/hello.mjs` runs after `npm install && npm run build`. npm publish landed in Phase 6g as `@joaoh82/sqlrite` (scoped — npm rejected the unscoped `sqlrite` name as too similar to `sqlite`). diff --git a/benchmarks/src/data.rs b/benchmarks/src/data.rs index f6f3247..b2e2b60 100644 --- a/benchmarks/src/data.rs +++ b/benchmarks/src/data.rs @@ -404,6 +404,11 @@ fn gen_vector(seed: u64, dim: usize) -> Vec { /// Render a `&[f32]` as the bracket-array literal SQLRite + the /// `[f32; 4]` example use: `[0.123, -0.456, …]`. +/// +/// SQLR-23 — no longer used by W10/W12 (those bind via +/// [`crate::Value::Vector`]); kept as a public helper for any future +/// workload that needs a SQL-string vector literal. +#[allow(dead_code)] pub fn vec_to_sql_literal(v: &[f32]) -> String { let mut s = String::with_capacity(v.len() * 12 + 2); s.push('['); diff --git a/benchmarks/src/drivers/duckdb.rs b/benchmarks/src/drivers/duckdb.rs index b99b3b3..9fa7962 100644 --- a/benchmarks/src/drivers/duckdb.rs +++ b/benchmarks/src/drivers/duckdb.rs @@ -49,7 +49,7 @@ impl Driver for DuckDBDriver { sql: &str, params: &[Value], ) -> Result<()> { - let bound: Vec = params.iter().map(to_duckdb).collect(); + let bound = to_duckdb_params(params)?; conn.execute(sql, duckdb::params_from_iter(bound.iter())) .with_context(|| format!("duckdb execute: {sql}"))?; Ok(()) @@ -70,7 +70,7 @@ impl Driver for DuckDBDriver { let mut stmt = conn .prepare(sql) .with_context(|| format!("duckdb prepare: {sql}"))?; - let bound: Vec = params.iter().map(to_duckdb).collect(); + let bound = to_duckdb_params(params)?; let mut rows = stmt .query(duckdb::params_from_iter(bound.iter())) .with_context(|| format!("duckdb query: {sql}"))?; @@ -102,7 +102,7 @@ impl Driver for DuckDBDriver { let mut stmt = conn .prepare(sql) .with_context(|| format!("duckdb prepare: {sql}"))?; - let bound: Vec = params.iter().map(to_duckdb).collect(); + let bound = to_duckdb_params(params)?; let mut rows = stmt .query(duckdb::params_from_iter(bound.iter())) .with_context(|| format!("duckdb query: {sql}"))?; @@ -125,12 +125,24 @@ impl Driver for DuckDBDriver { } } -fn to_duckdb(v: &Value) -> duckdb::types::Value { +fn to_duckdb_params(params: &[Value]) -> Result> { + params.iter().map(to_duckdb).collect() +} + +fn to_duckdb(v: &Value) -> Result { match v { - Value::Null => duckdb::types::Value::Null, - Value::Integer(i) => duckdb::types::Value::BigInt(*i), - Value::Real(f) => duckdb::types::Value::Double(*f), - Value::Text(s) => duckdb::types::Value::Text(s.clone()), + Value::Null => Ok(duckdb::types::Value::Null), + Value::Integer(i) => Ok(duckdb::types::Value::BigInt(*i)), + Value::Real(f) => Ok(duckdb::types::Value::Double(*f)), + Value::Text(s) => Ok(duckdb::types::Value::Text(s.clone())), + // VECTOR is SQLRite-only (DuckDB doesn't ship a comparable + // primitive). The Group C workloads gate on + // `driver_supports("sqlrite")`, so reaching this arm + // indicates a registration bug. + Value::Vector(_) => anyhow::bail!( + "duckdb driver: VECTOR params are SQLRite-only; this workload should not register \ + against duckdb" + ), } } diff --git a/benchmarks/src/drivers/sqlite.rs b/benchmarks/src/drivers/sqlite.rs index 048e2be..47f2588 100644 --- a/benchmarks/src/drivers/sqlite.rs +++ b/benchmarks/src/drivers/sqlite.rs @@ -64,7 +64,7 @@ impl Driver for SQLiteDriver { sql: &str, params: &[Value], ) -> Result<()> { - let bound: Vec = params.iter().map(to_rusqlite).collect(); + let bound = to_rusqlite_params(params)?; conn.execute(sql, rusqlite::params_from_iter(bound.iter())) .with_context(|| format!("rusqlite execute: {sql}"))?; Ok(()) @@ -79,7 +79,7 @@ impl Driver for SQLiteDriver { let mut stmt = conn .prepare_cached(sql) .with_context(|| format!("rusqlite prepare_cached: {sql}"))?; - let bound: Vec = params.iter().map(to_rusqlite).collect(); + let bound = to_rusqlite_params(params)?; let cols = stmt.column_count(); let mut rows = stmt .query(rusqlite::params_from_iter(bound.iter())) @@ -111,7 +111,7 @@ impl Driver for SQLiteDriver { let mut stmt = conn .prepare_cached(sql) .with_context(|| format!("rusqlite prepare_cached: {sql}"))?; - let bound: Vec = params.iter().map(to_rusqlite).collect(); + let bound = to_rusqlite_params(params)?; let cols = stmt.column_count(); let mut rows = stmt .query(rusqlite::params_from_iter(bound.iter())) @@ -131,12 +131,24 @@ impl Driver for SQLiteDriver { } } -fn to_rusqlite(v: &Value) -> rusqlite::types::Value { +fn to_rusqlite_params(params: &[Value]) -> Result> { + params.iter().map(to_rusqlite).collect() +} + +fn to_rusqlite(v: &Value) -> Result { match v { - Value::Null => rusqlite::types::Value::Null, - Value::Integer(i) => rusqlite::types::Value::Integer(*i), - Value::Real(f) => rusqlite::types::Value::Real(*f), - Value::Text(s) => rusqlite::types::Value::Text(s.clone()), + Value::Null => Ok(rusqlite::types::Value::Null), + Value::Integer(i) => Ok(rusqlite::types::Value::Integer(*i)), + Value::Real(f) => Ok(rusqlite::types::Value::Real(*f)), + Value::Text(s) => Ok(rusqlite::types::Value::Text(s.clone())), + // VECTOR is SQLRite-only; the W10/W12 workloads gate on + // `driver_supports("sqlrite")` so this branch indicates a + // bug in workload registration. Fail loudly rather than + // silently coercing. + Value::Vector(_) => anyhow::bail!( + "rusqlite driver: VECTOR params are SQLRite-only; this workload should not register \ + against sqlite" + ), } } diff --git a/benchmarks/src/drivers/sqlrite.rs b/benchmarks/src/drivers/sqlrite.rs index 3ecff55..6c68fe2 100644 --- a/benchmarks/src/drivers/sqlrite.rs +++ b/benchmarks/src/drivers/sqlrite.rs @@ -1,16 +1,26 @@ //! SQLRite driver. //! //! Binds against the engine's public [`sqlrite::Connection`] surface — -//! the same API the language SDKs use. SQLRite has no parameter -//! binding yet (see `connection.rs:145` — "parameter binding and -//! prepared-plan caching are future work"), so the driver formats -//! `[Value]` into the SQL string at call time. That's an honest cost -//! to include in the comparison: a SQLRite user calling a hot SELECT -//! today pays the same per-call parse + format overhead. +//! the same API the language SDKs use. //! -//! Once SQLRite gains parameter binding (post-9.6 follow-up), this -//! driver will switch to the bound path and a workload `v` bump will -//! capture the methodology change. +//! ## SQLR-23 — bound + cached path +//! +//! SQLRite gained a prepared-statement plan cache + parameter binding +//! in SQLR-23. This driver uses both: +//! +//! - `query_one` / `query_all` route through [`sqlrite::Connection::prepare_cached`] +//! so a hot SELECT pays the sqlparser walk exactly once across the +//! whole bench loop (cache cap defaults to 16, plenty for any single +//! workload). +//! - `execute_with_params` does the same for INSERT-loop hot paths. +//! - `Value::Vector` binds directly through `Statement::query_with_params` +//! without round-tripping through a 4 KB bracket-array SQL literal — +//! this is the W10/W12 unlock. The HNSW probe optimizer recognizes +//! the bound vector via the same in-band shape an inline `[…]` would +//! produce, so the optimizer hook still kicks in on bound queries. +//! +//! That's how a perf-conscious SQLRite user would write hot-path code +//! today. use std::path::Path; @@ -44,20 +54,23 @@ impl Driver for SQLRiteDriver { sql: &str, params: &[Value], ) -> Result<()> { - let inlined = inline_params(sql, params)?; - conn.execute(&inlined) - .map_err(|e| anyhow::anyhow!("sqlrite execute_with_params: {e}\n sql: {inlined}"))?; + let bound = to_engine_values(params); + let mut stmt = conn + .prepare_cached(sql) + .map_err(|e| anyhow::anyhow!("sqlrite prepare_cached: {e}\n sql: {sql}"))?; + stmt.execute_with_params(&bound) + .map_err(|e| anyhow::anyhow!("sqlrite execute_with_params: {e}\n sql: {sql}"))?; Ok(()) } fn query_one(&self, conn: &mut Self::Conn, sql: &str, params: &[Value]) -> Result> { - let inlined = inline_params(sql, params)?; + let bound = to_engine_values(params); let stmt = conn - .prepare(&inlined) - .map_err(|e| anyhow::anyhow!("sqlrite prepare: {e}\n sql: {inlined}"))?; + .prepare_cached(sql) + .map_err(|e| anyhow::anyhow!("sqlrite prepare_cached: {e}\n sql: {sql}"))?; let mut rows = stmt - .query() - .map_err(|e| anyhow::anyhow!("sqlrite query: {e}\n sql: {inlined}"))?; + .query_with_params(&bound) + .map_err(|e| anyhow::anyhow!("sqlrite query_with_params: {e}\n sql: {sql}"))?; let row = rows .next() .map_err(|e| anyhow::anyhow!("sqlrite row read: {e}"))? @@ -84,13 +97,13 @@ impl Driver for SQLRiteDriver { sql: &str, params: &[Value], ) -> Result>> { - let inlined = inline_params(sql, params)?; + let bound = to_engine_values(params); let stmt = conn - .prepare(&inlined) - .map_err(|e| anyhow::anyhow!("sqlrite prepare: {e}\n sql: {inlined}"))?; + .prepare_cached(sql) + .map_err(|e| anyhow::anyhow!("sqlrite prepare_cached: {e}\n sql: {sql}"))?; let mut rows = stmt - .query() - .map_err(|e| anyhow::anyhow!("sqlrite query: {e}\n sql: {inlined}"))?; + .query_with_params(&bound) + .map_err(|e| anyhow::anyhow!("sqlrite query_with_params: {e}\n sql: {sql}"))?; let mut out = Vec::new(); while let Some(row) = rows .next() @@ -108,49 +121,19 @@ impl Driver for SQLRiteDriver { } } -/// Inline `?`-positional placeholders with literal values. Replaces the -/// first `?` with `params[0]`, the second with `params[1]`, etc. Errors -/// if the count doesn't match. Strings are SQL-escaped. -fn inline_params(sql: &str, params: &[Value]) -> Result { - let mut out = String::with_capacity(sql.len() + params.len() * 16); - let mut iter = params.iter(); - let mut in_string = false; - for ch in sql.chars() { - if ch == '\'' { - in_string = !in_string; - out.push(ch); - continue; - } - if ch == '?' && !in_string { - let p = iter - .next() - .context("inline_params: more `?` placeholders than params")?; - push_literal(&mut out, p); - } else { - out.push(ch); - } - } - if iter.next().is_some() { - anyhow::bail!("inline_params: more params than `?` placeholders"); - } - Ok(out) +/// Map the bench harness's `Value` to SQLRite's engine `Value`. Both +/// enums carry the same logical shapes; this is just a name-mapping. +fn to_engine_values(params: &[Value]) -> Vec { + params.iter().map(to_engine_value).collect() } -fn push_literal(out: &mut String, v: &Value) { +fn to_engine_value(v: &Value) -> sqlrite::Value { match v { - Value::Null => out.push_str("NULL"), - Value::Integer(i) => out.push_str(&i.to_string()), - Value::Real(f) => out.push_str(&format!("{f}")), - Value::Text(s) => { - out.push('\''); - for ch in s.chars() { - if ch == '\'' { - out.push('\''); - } - out.push(ch); - } - out.push('\''); - } + Value::Null => sqlrite::Value::Null, + Value::Integer(i) => sqlrite::Value::Integer(*i), + Value::Real(f) => sqlrite::Value::Real(*f), + Value::Text(s) => sqlrite::Value::Text(s.clone()), + Value::Vector(v) => sqlrite::Value::Vector(v.clone()), } } @@ -160,46 +143,10 @@ fn from_engine_value(v: sqlrite::Value) -> Value { sqlrite::Value::Integer(i) => Value::Integer(i), sqlrite::Value::Real(f) => Value::Real(f), sqlrite::Value::Text(s) => Value::Text(s), - // Bench inputs don't include booleans / vectors / JSON yet — - // when a workload starts using them, this match grows. + sqlrite::Value::Vector(v) => Value::Vector(v), + // Bool / JSON aren't yet a bench `Value` variant — workloads + // don't surface them. If a future workload reads one back, + // grow this match alongside the harness `Value` enum. other => Value::Text(format!("{other:?}")), } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn inline_params_replaces_in_order() { - let s = inline_params( - "SELECT * FROM t WHERE a = ? AND b = ? AND c = ?", - &[Value::Integer(1), Value::Text("x".into()), Value::Null], - ) - .unwrap(); - assert_eq!(s, "SELECT * FROM t WHERE a = 1 AND b = 'x' AND c = NULL"); - } - - #[test] - fn inline_params_preserves_question_marks_in_strings() { - let s = - inline_params("SELECT 'what?', * FROM t WHERE a = ?", &[Value::Integer(7)]).unwrap(); - assert_eq!(s, "SELECT 'what?', * FROM t WHERE a = 7"); - } - - #[test] - fn inline_params_escapes_quotes() { - let s = inline_params( - "SELECT * FROM t WHERE name = ?", - &[Value::Text("O'Hara".into())], - ) - .unwrap(); - assert_eq!(s, "SELECT * FROM t WHERE name = 'O''Hara'"); - } - - #[test] - fn inline_params_arity_mismatch_errors() { - assert!(inline_params("SELECT ?", &[]).is_err()); - assert!(inline_params("SELECT 1", &[Value::Integer(1)]).is_err()); - } -} diff --git a/benchmarks/src/lib.rs b/benchmarks/src/lib.rs index 9f37301..e6149f1 100644 --- a/benchmarks/src/lib.rs +++ b/benchmarks/src/lib.rs @@ -41,18 +41,27 @@ pub mod workloads; /// Driver-side value type. Tight enough that any of the engines under /// test can map it onto their native bind types — rusqlite has -/// [`rusqlite::ToSql`], DuckDB has its own; SQLRite has no parameter -/// binding yet so the SQLRite driver inlines via SQL formatting. +/// [`rusqlite::ToSql`], DuckDB has its own. SQLRite gained parameter +/// binding in SQLR-23 (incl. `Value::Vector` for HNSW-eligible KNN +/// queries), so the SQLRite driver now binds through +/// `Statement::query_with_params` / `Statement::execute_with_params` +/// instead of formatting a SQL string per call. /// -/// Deliberately doesn't carry every type the engines support -/// (booleans, vectors, JSON, blobs); workload inputs only need these -/// four. New variants land alongside the workload that needs them. +/// `Vector` is SQLRite-only: SQLite-side drivers raise a clean error +/// rather than silently lying about the type, since the W10/W12 +/// workloads that exercise it are explicitly SQLRite-only via +/// `driver_supports`. #[derive(Debug, Clone, PartialEq)] pub enum Value { Null, Integer(i64), Real(f64), Text(String), + /// Dense f32 query vector — bound directly into VECTOR columns + /// or distance-function args. SQLRite-only; comparator drivers + /// surface a typed error if a workload tries to bind a vector + /// against them. + Vector(Vec), } /// Engine-agnostic surface every workload binds to. diff --git a/benchmarks/src/workloads/aggregate.rs b/benchmarks/src/workloads/aggregate.rs index 7aa986b..54058c8 100644 --- a/benchmarks/src/workloads/aggregate.rs +++ b/benchmarks/src/workloads/aggregate.rs @@ -18,7 +18,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W7: WorkloadId = WorkloadId { id: "W7", name: "aggregate-sum", - version: "v1", + version: "v2", }; pub const SELECT_SQL: &str = "SELECT SUM(v) FROM big"; diff --git a/benchmarks/src/workloads/bulk_insert.rs b/benchmarks/src/workloads/bulk_insert.rs index adafdd1..2d31759 100644 --- a/benchmarks/src/workloads/bulk_insert.rs +++ b/benchmarks/src/workloads/bulk_insert.rs @@ -37,7 +37,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W3: WorkloadId = WorkloadId { id: "W3", name: "bulk-insert", - version: "v1", + version: "v2", }; /// One per-iter dataset is reused across samples — the row contents diff --git a/benchmarks/src/workloads/fts.rs b/benchmarks/src/workloads/fts.rs index 1bfef6f..431886d 100644 --- a/benchmarks/src/workloads/fts.rs +++ b/benchmarks/src/workloads/fts.rs @@ -47,12 +47,24 @@ use anyhow::{Context, Result}; use crate::data::{FTS_QUERY_COUNT, FTS_ROW_COUNT, FtsDataset, fts_dataset}; use crate::{Driver, Value, WorkloadId}; +/// SQLR-23 — bumped to v2 because the SQLRite path now binds the FTS +/// query string through `?` placeholders (twice: once for `fts_match`, +/// once for `bm25_score`) instead of inlining + escaping at call time. +/// The "parser tax" the bench README has called out since 9.1 +/// disappears for this workload. pub const W11: WorkloadId = WorkloadId { id: "W11", name: "bm25-top10", - version: "v1", + version: "v2", }; +/// SQLRite-side hot-loop SQL — both `fts_match` and `bm25_score` +/// reference the same query string via two `?` placeholders. Static +/// across iterations so `prepare_cached` returns the same plan +/// every call. +const SQLRITE_SELECT_SQL: &str = + "SELECT id FROM docs WHERE fts_match(body, ?) ORDER BY bm25_score(body, ?) DESC LIMIT 10"; + pub fn setup(driver: &D, path: &Path) -> Result<(D::Conn, FtsDataset)> { let mut conn = driver.open(path)?; create_schema(driver, &mut conn)?; @@ -128,16 +140,20 @@ pub fn bench_iter(driver: &D, conn: &mut D::Conn, query: &str) -> Res )? } _ => { - // SQLRite: fts_match filters, bm25_score ranks. The query - // string appears twice — once in the WHERE filter, once in - // the ORDER BY ranker (matches the engine's API; both - // calls share an internal cache per `docs/fts.md`). - let sql = format!( - "SELECT id FROM docs WHERE fts_match(body, '{}') ORDER BY bm25_score(body, '{}') DESC LIMIT 10", - escape_sql(query), - escape_sql(query), - ); - driver.query_all(conn, &sql, &[])? + // SQLRite: fts_match filters, bm25_score ranks. SQLR-23 — + // bind the query string twice through two `?` slots, so the + // SQL template stays static and `prepare_cached` reuses the + // same parsed plan across every iteration. Both calls + // still share the engine's internal per-query FTS cache + // per `docs/fts.md`. + driver.query_all( + conn, + SQLRITE_SELECT_SQL, + &[ + Value::Text(query.to_string()), + Value::Text(query.to_string()), + ], + )? } }; Ok(rows.len()) @@ -162,7 +178,3 @@ pub fn correctness_check( } Ok(()) } - -fn escape_sql(s: &str) -> String { - s.replace('\'', "''") -} diff --git a/benchmarks/src/workloads/group_by.rs b/benchmarks/src/workloads/group_by.rs index 20914d5..2b8ac29 100644 --- a/benchmarks/src/workloads/group_by.rs +++ b/benchmarks/src/workloads/group_by.rs @@ -29,7 +29,7 @@ use crate::{Driver, WorkloadId}; pub const W8: WorkloadId = WorkloadId { id: "W8", name: "group-by", - version: "v1", + version: "v2", }; /// Cardinality buckets. `(label, group-key column, expected group count)`. diff --git a/benchmarks/src/workloads/hybrid.rs b/benchmarks/src/workloads/hybrid.rs index 4a1c815..55fe453 100644 --- a/benchmarks/src/workloads/hybrid.rs +++ b/benchmarks/src/workloads/hybrid.rs @@ -40,17 +40,30 @@ use std::path::Path; use anyhow::{Context, Result}; -use crate::data::{ - FTS_ROW_COUNT, FtsDataset, VectorDataset, fts_dataset, vec_to_sql_literal, vector_dataset, -}; +use crate::data::{FTS_ROW_COUNT, FtsDataset, VectorDataset, fts_dataset, vector_dataset}; use crate::{Driver, Value, WorkloadId}; +/// SQLR-23 — bumped to v2 because the hybrid SQL is now fully +/// parameterized (FTS query string twice + query vector once) instead +/// of formatted into the SQL string per call. The static template +/// hits the prepared-plan cache. pub const W12: WorkloadId = WorkloadId { id: "W12", name: "hybrid", - version: "v1", + version: "v2", }; +/// Hot-loop SQL for the hybrid query — three `?` placeholders: +/// `?1` and `?2` are the FTS query string (used for both filter and +/// ranker), `?3` is the cosine query vector. Static across iterations. +pub const SELECT_SQL: &str = "SELECT id FROM docs \ + WHERE fts_match(body, ?) \ + ORDER BY 0.5 * (1.0 - bm25_score(body, ?) / 10.0) + 0.5 * vec_distance_cosine(embedding, ?) \ + ASC LIMIT 10"; + +/// Insert SQL for the seed pass — id, body, embedding all bound. +pub const INSERT_SQL: &str = "INSERT INTO docs (id, body, embedding) VALUES (?, ?, ?)"; + pub struct HybridDataset { pub fts: FtsDataset, pub vec: VectorDataset, @@ -75,15 +88,19 @@ pub fn bench_iter( text_query: &str, vec_query: &[f32], ) -> Result { - let lit = vec_to_sql_literal(vec_query); - let q = escape_sql(text_query); - let sql = format!( - "SELECT id FROM docs \ - WHERE fts_match(body, '{q}') \ - ORDER BY 0.5 * (1.0 - bm25_score(body, '{q}') / 10.0) + 0.5 * vec_distance_cosine(embedding, {lit}) \ - ASC LIMIT 10" - ); - let rows = driver.query_all(conn, &sql, &[])?; + // SQLR-23 — every component bound through `?`. The SQL template is + // identical across the 64 random query pairs, so the + // prepare_cached LRU keeps a single plan hot for the entire bench + // loop. + let rows = driver.query_all( + conn, + SELECT_SQL, + &[ + Value::Text(text_query.to_string()), + Value::Text(text_query.to_string()), + Value::Vector(vec_query.to_vec()), + ], + )?; Ok(rows.len()) } @@ -125,13 +142,17 @@ fn insert_rows( driver.execute(conn, "BEGIN").context("W12 BEGIN")?; for (f, v) in fts.rows.iter().zip(vec.rows.iter()) { debug_assert_eq!(f.id, v.id); - let lit = vec_to_sql_literal(&v.embedding); - let sql = format!("INSERT INTO docs (id, body, embedding) VALUES (?, ?, {lit})"); + // SQLR-23 — id / body / embedding all bound. Same plan is + // cached and reused across every row of the seed pass. driver .execute_with_params( conn, - &sql, - &[Value::Integer(f.id), Value::Text(f.body.clone())], + INSERT_SQL, + &[ + Value::Integer(f.id), + Value::Text(f.body.clone()), + Value::Vector(v.embedding.clone()), + ], ) .with_context(|| format!("W12 INSERT id={}", f.id))?; } @@ -139,7 +160,3 @@ fn insert_rows( debug_assert_eq!(fts.rows.len(), FTS_ROW_COUNT); Ok(()) } - -fn escape_sql(s: &str) -> String { - s.replace('\'', "''") -} diff --git a/benchmarks/src/workloads/index_lookup.rs b/benchmarks/src/workloads/index_lookup.rs index 4537204..dad25b6 100644 --- a/benchmarks/src/workloads/index_lookup.rs +++ b/benchmarks/src/workloads/index_lookup.rs @@ -26,7 +26,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W6: WorkloadId = WorkloadId { id: "W6", name: "index-lookup", - version: "v1", + version: "v2", }; pub const SELECT_SQL: &str = "SELECT id, payload FROM kv2 WHERE secondary = ?"; diff --git a/benchmarks/src/workloads/join.rs b/benchmarks/src/workloads/join.rs index 8bdeb92..29f75c5 100644 --- a/benchmarks/src/workloads/join.rs +++ b/benchmarks/src/workloads/join.rs @@ -50,7 +50,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W9: WorkloadId = WorkloadId { id: "W9", name: "inner-join", - version: "v1", + version: "v2", }; pub const SELECT_SQL: &str = "SELECT c.id, c.name, o.amount FROM customers AS c INNER JOIN orders AS o ON c.id = o.customer_id WHERE c.id = ?"; diff --git a/benchmarks/src/workloads/kv.rs b/benchmarks/src/workloads/kv.rs index ab75061..d02032d 100644 --- a/benchmarks/src/workloads/kv.rs +++ b/benchmarks/src/workloads/kv.rs @@ -30,7 +30,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W1: WorkloadId = WorkloadId { id: "W1", name: "read-by-pk", - version: "v1", + version: "v2", }; /// SELECT used by the hot loop. Both engines support `?`-positional diff --git a/benchmarks/src/workloads/mixed_oltp.rs b/benchmarks/src/workloads/mixed_oltp.rs index d1be8d3..67db483 100644 --- a/benchmarks/src/workloads/mixed_oltp.rs +++ b/benchmarks/src/workloads/mixed_oltp.rs @@ -31,7 +31,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W5: WorkloadId = WorkloadId { id: "W5", name: "mixed-oltp", - version: "v1", + version: "v2", }; pub const SELECT_SQL: &str = "SELECT id, secondary, payload FROM kv2 WHERE id = ?"; diff --git a/benchmarks/src/workloads/range_scan.rs b/benchmarks/src/workloads/range_scan.rs index e930f6d..3c0e179 100644 --- a/benchmarks/src/workloads/range_scan.rs +++ b/benchmarks/src/workloads/range_scan.rs @@ -37,7 +37,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W2: WorkloadId = WorkloadId { id: "W2", name: "range-scan", - version: "v1", + version: "v2", }; pub const SELECT_SQL: &str = diff --git a/benchmarks/src/workloads/single_insert.rs b/benchmarks/src/workloads/single_insert.rs index 0adf8e7..35bd246 100644 --- a/benchmarks/src/workloads/single_insert.rs +++ b/benchmarks/src/workloads/single_insert.rs @@ -43,7 +43,7 @@ use crate::{Driver, Value, WorkloadId}; pub const W4: WorkloadId = WorkloadId { id: "W4", name: "single-insert", - version: "v1", + version: "v2", }; /// Setup: open fresh DB, create `kv_writes`, preload [`W4_PRELOAD_ROWS`] diff --git a/benchmarks/src/workloads/vector.rs b/benchmarks/src/workloads/vector.rs index 02fe6b3..05b8302 100644 --- a/benchmarks/src/workloads/vector.rs +++ b/benchmarks/src/workloads/vector.rs @@ -32,20 +32,32 @@ use std::path::Path; use anyhow::{Context, Result}; -use crate::data::{ - VECTOR_QUERY_COUNT, VECTOR_ROW_COUNT, VectorDataset, vec_to_sql_literal, vector_dataset, -}; +use crate::data::{VECTOR_QUERY_COUNT, VECTOR_ROW_COUNT, VectorDataset, vector_dataset}; use crate::{Driver, Value, WorkloadId}; +/// SQLR-23 — bumped to v2 because the bench-driver methodology changed: +/// the query vector is now bound through `Value::Vector` instead of +/// inlined as a 4 KB bracket-array literal in the SQL string. The +/// brute-force-vs-HNSW gap should widen materially because the +/// per-iter parser cost no longer dominates. pub const W10: WorkloadId = WorkloadId { id: "W10", name: "vector-top10", - version: "v1", + version: "v2", }; /// `(label, with_hnsw_index)` — two variants per driver. pub const VARIANTS: [(&str, bool); 2] = [("brute-force", false), ("hnsw", true)]; +/// Hot-loop SQL — fully parameterized: the embedding column gets +/// bound to a `Value::Vector(query)`. Static across iterations so +/// `prepare_cached` returns the same plan every call. +pub const SELECT_SQL: &str = + "SELECT id FROM vecs ORDER BY vec_distance_cosine(embedding, ?) ASC LIMIT 10"; + +/// Insert SQL for the seed pass — id and embedding both bound. +pub const INSERT_SQL: &str = "INSERT INTO vecs (id, embedding) VALUES (?, ?)"; + pub fn setup( driver: &D, path: &Path, @@ -69,11 +81,12 @@ pub fn setup( /// One iteration: top-10 cosine-nearest probes for `query`. Returns /// the row count so criterion's black_box has a stable fingerprint. +/// +/// SQLR-23: `query` binds through `Value::Vector` instead of being +/// formatted into the SQL string. With the vector out of the lexer's +/// hot path, the HNSW probe optimizer becomes visible vs brute-force. pub fn bench_iter(driver: &D, conn: &mut D::Conn, query: &[f32]) -> Result { - let lit = vec_to_sql_literal(query); - let sql = - format!("SELECT id FROM vecs ORDER BY vec_distance_cosine(embedding, {lit}) ASC LIMIT 10"); - let rows = driver.query_all(conn, &sql, &[])?; + let rows = driver.query_all(conn, SELECT_SQL, &[Value::Vector(query.to_vec())])?; Ok(rows.len()) } @@ -102,13 +115,14 @@ pub fn driver_supports(driver_name: &str) -> bool { fn insert_rows(driver: &D, conn: &mut D::Conn, dataset: &VectorDataset) -> Result<()> { driver.execute(conn, "BEGIN").context("W10 BEGIN")?; for row in &dataset.rows { - let lit = vec_to_sql_literal(&row.embedding); - // Inline the vector literal directly — there's no `?`-bind for - // VECTOR values in SQLRite's current public API. Driver-side - // params handle id only. - let sql = format!("INSERT INTO vecs (id, embedding) VALUES (?, {lit})"); + // SQLR-23 — both id and embedding are now bound. Same + // `prepare_cached` plan reused for every row. driver - .execute_with_params(conn, &sql, &[Value::Integer(row.id)]) + .execute_with_params( + conn, + INSERT_SQL, + &[Value::Integer(row.id), Value::Vector(row.embedding.clone())], + ) .with_context(|| format!("W10 INSERT id={}", row.id))?; } driver.execute(conn, "COMMIT").context("W10 COMMIT")?; diff --git a/docs/architecture.md b/docs/architecture.md index cbc8797..032e971 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -90,12 +90,13 @@ The engine never depends on the SDK crates; the SDK crates each depend on the en |---|---| | [`src/main.rs`](../src/main.rs) | Binary entry: init env_logger, build rustyline editor, run the REPL loop, route input to either the meta or SQL dispatcher | | [`src/lib.rs`](../src/lib.rs) | Library entry: re-exports `Connection`, `Statement`, `Rows`, `Value`, `Database`, `process_command` / `process_command_with_render` / `CommandOutput`, the `ask` module (when feature on), etc. — the stable public surface every SDK binds against | -| [`src/connection.rs`](../src/connection.rs) | `Connection` / `Statement` / `Rows` / `Row` / `OwnedRow` / `FromValue` — the Phase 5a public API | +| [`src/connection.rs`](../src/connection.rs) | `Connection` / `Statement` / `Rows` / `Row` / `OwnedRow` / `FromValue` — the Phase 5a public API. SQLR-23 added the per-connection `prepare_cached` LRU + the bound `Statement::query_with_params` / `execute_with_params` (parsed AST cached on the `Statement`; `?` placeholders substituted at execute time without re-running sqlparser). | +| [`src/sql/params.rs`](../src/sql/params.rs) | SQLR-23 — placeholder rewriter (`?` → `?N` source-order numbering at prepare time) and AST substitution pass that lowers `&[Value]` into the same in-band literal shapes the executor already recognizes (scalars become `Expr::Value(...)`, vectors become bracket-array `Expr::Identifier`). Used by `Statement::{query,execute}_with_params`. | | [`src/ask/`](../src/ask/) | Engine integration with `sqlrite-ask`: `ConnectionAskExt`, `ask_with_database`, the `schema::dump_schema_for_database` helper. The `schema` submodule is always available; the rest is gated behind the `ask` feature. Phase 7g.2. | | [`src/repl/`](../src/repl/) | `REPLHelper` (implements rustyline's `Helper` trait: completer, hinter, highlighter, validator). Also `get_config` and `get_command_type` | | [`src/meta_command/`](../src/meta_command/) | `MetaCommand` enum, parsing (`.open FOO.db` → `Open(PathBuf)`, `.ask ` → `Ask(String)`), and dispatch to persistence + ask helpers | | [`src/error.rs`](../src/error.rs) | `SQLRiteError` (thiserror-derived), `Result` alias, hand-rolled `PartialEq` that handles `io::Error` | -| [`src/sql/mod.rs`](../src/sql/mod.rs) | `SQLCommand` classifier, `process_command` / `process_command_with_render` — the top-level entries that parse a SQL string and route to the right executor. Also triggers auto-save. **Never writes to stdout** — for SELECT statements, the rendered prettytable comes back inside `CommandOutput.rendered` so the REPL can print it (the engine itself doesn't); the SDK / FFI / MCP callers ignore it. | +| [`src/sql/mod.rs`](../src/sql/mod.rs) | `SQLCommand` classifier, `process_command` / `process_command_with_render` — the top-level entries that parse a SQL string and route to the right executor. SQLR-23 added `process_ast_with_render(stmt, db)` for callers (the `Statement` API) that already hold a parsed AST and want to skip the sqlparser walk. Also triggers auto-save. **Never writes to stdout** — for SELECT statements, the rendered prettytable comes back inside `CommandOutput.rendered` so the REPL can print it (the engine itself doesn't); the SDK / FFI / MCP callers ignore it. | | [`src/sql/parser/`](../src/sql/parser/) | Takes a `sqlparser::ast::Statement` and produces internal query structs (`CreateQuery`, `InsertQuery`, `SelectQuery`) with only the fields we actually use | | [`src/sql/executor.rs`](../src/sql/executor.rs) | `execute_select`, `execute_delete`, `execute_update`, plus the shared expression evaluator `eval_expr` / `eval_predicate`. Also the bounded-heap top-k optimization (Phase 7c), the HNSW probe shortcut (Phase 7d.2), and the FTS probe shortcut (Phase 8b). | | [`src/sql/db/database.rs`](../src/sql/db/database.rs) | `Database`: table map + optional `source_path` + optional long-lived `Pager` + transaction-snapshot state | diff --git a/docs/benchmarks-plan.md b/docs/benchmarks-plan.md index 5a8e8ca..077ff61 100644 --- a/docs/benchmarks-plan.md +++ b/docs/benchmarks-plan.md @@ -349,7 +349,7 @@ If we add a column or change a query in a workload between releases, how do we k ## Risks + things to watch -- **Driver bias.** A poorly-written SQLite driver call (e.g. forgetting `prepare_cached`) makes SQLRite look 5× better than it is. Mitigation: code review every driver impl with the question "is this how a perf-conscious user of would write it?", and the correctness gate (hash-matching) catches divergent semantics. +- **Driver bias.** A poorly-written SQLite driver call (e.g. forgetting `prepare_cached`) makes SQLRite look 5× better than it is. Mitigation: code review every driver impl with the question "is this how a perf-conscious user of would write it?", and the correctness gate (hash-matching) catches divergent semantics. Update — SQLR-23: SQLRite gained `prepare_cached` + parameter binding, and the bench harness's SQLRite driver was flipped from per-call SQL formatting (`inline_params`) to the bound + cached path. Every workload's `WorkloadId.version` was bumped `v1 → v2` in lockstep so this methodology change is captured explicitly. Old `v1` JSON envelopes stay readable but the comparison script flags cross-version pairs. - **Criterion overhead in micro-workloads.** For W1 (sub-microsecond per op territory after warmup), criterion's per-iter accounting can dominate. Mitigation: batch iterations inside the bench closure (criterion's `iter_batched` + a 1k-iteration inner loop), report ops/s computed against the inner-loop count. - **`sqlite-vec` availability.** The extension isn't shipped with stock SQLite. W10 should treat the SQLite vector comparator as opportunistic — if not installed, run SQLRite-only and note it in the table. Don't make Group C hard-depend on it. - **macOS vs Linux skew.** fsync semantics differ; W4 numbers won't be portable across OSes. Mitigation: JSON envelope captures `os.kind`, results page only compares within the same OS family. diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 97290aa..aed9c68 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -82,12 +82,15 @@ A few methodology notes that change how you read the table. **Q3 — DuckDB runs at defaults.** No equivalent to SQLite's WAL+NORMAL knob; DuckDB's MVCC + commit semantics are uniform. -**Where SQLRite still pays a parser tax.** The engine doesn't ship parameter binding or a prepared-plan cache yet — every iteration walks the full `sqlparser` AST again. Several workloads' headline numbers are dominated by this overhead, not the underlying execution path. Examples: +**Parser tax — historical, addressed in SQLR-23.** Pre-SQLR-23, the engine parsed SQL on every `Connection::execute` / `Connection::prepare` call. The bench driver's SQLRite path inlined `?` placeholders into the SQL string, so every iteration also walked the full `sqlparser` AST. Several workloads' headline numbers were dominated by this overhead — W1 and W6 (sub-µs paths where parser cost was most of the per-iter time), W10 (the 384-dim bracket-array literal in `ORDER BY` was ~4 KB of SQL the parser walked every iteration; brute-force vs HNSW looked indistinguishable as a result). -- W1 / W6 — sub-µs paths where parser cost is most of the per-iter time. -- W10 — the 384-dim bracket-array literal in `ORDER BY` is ~4 KB of SQL the parser walks every iteration; brute-force vs HNSW look indistinguishable as a result. +[SQLR-23](https://github.com/joaoh82/rust_sqlite/pulls?q=SQLR-23) shipped: -A future "prepared statement support" follow-up is the unlock for several rows of this table. +- `Connection::prepare_cached` — small per-connection LRU of parsed plans (default cap 16, matches rusqlite). +- `Statement::query_with_params(&[Value])` / `Statement::execute_with_params(&[Value])` — bind `?` placeholders at execute time without re-running sqlparser. +- `Value::Vector(Vec)` as a first-class bind type — the 4 KB query vector for W10 is now bound directly instead of being re-lexed every iteration. The HNSW probe optimizer still recognizes the bound shape, so the algorithmic shortcut keeps firing. + +The bench harness `Driver::query_one` / `query_all` paths route through `prepare_cached` + the bound API. Every workload's `WorkloadId.version` was bumped `v1 → v2` in lockstep — old JSON envelopes keep the v1 tag and stay readable, but cross-version comparisons require an explicit acknowledgment in the comparison script. The next official pinned-host run will land the post-binding numbers; treat the v1 row above as "before" and watch this section for the "after" once republished. **Where DuckDB is misleading.** Per-PK-probe single-row OLTP queries (W9) are SQLite's home turf, not DuckDB's. The plan flags this as "apples-to-oranges"; we still publish the number because the directional comparison is informative. diff --git a/docs/supported-sql.md b/docs/supported-sql.md index 6c3a35a..4d9d9cc 100644 --- a/docs/supported-sql.md +++ b/docs/supported-sql.md @@ -21,6 +21,32 @@ If you're looking for _how_ to use SQLRite (REPL flow, meta-commands, history, e Statements the parser accepts (because sqlparser understands them in the SQLite dialect) but SQLRite doesn't execute yet return `SQL Statement not supported yet`. The [Not yet supported](#not-yet-supported) section below enumerates the common ones. +### Parameter binding (SQLR-23) + +Every statement above accepts `?` placeholders anywhere a value literal is allowed (WHERE, ORDER BY, INSERT VALUES, …). Bind via the public Rust API: + +```rust +use sqlrite::{Connection, Value}; + +let mut conn = Connection::open_in_memory()?; +conn.execute("CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT, age INTEGER)")?; + +let mut ins = conn.prepare_cached("INSERT INTO users (name, age) VALUES (?, ?)")?; +ins.execute_with_params(&[Value::Text("alice".into()), Value::Integer(30)])?; +ins.execute_with_params(&[Value::Text("bob".into()), Value::Integer(25)])?; + +let stmt = conn.prepare_cached("SELECT name FROM users WHERE age > ?")?; +let rows = stmt + .query_with_params(&[Value::Integer(26)])? + .collect_all()?; +# Ok::<(), sqlrite::SQLRiteError>(()) +``` + +- **Positional only.** `?` placeholders are bound by source-order index (`params[0]` = first `?`, etc.). Named placeholders (`:foo`, `$1`) are not yet supported. +- **Strict arity.** The slice length must match the placeholder count or `query_with_params` / `execute_with_params` returns a typed error. +- **Vector binding.** `Value::Vector(Vec)` binds where a bracket-array literal would normally appear — including the second arg of `vec_distance_*` inside an HNSW-eligible `ORDER BY`. The optimizer recognizes the bound shape, so the graph shortcut still fires. +- **Plan cache.** `Connection::prepare_cached` keeps a per-connection LRU (default cap 16; tune via `set_prepared_cache_capacity`) so a hot SQL string parses exactly once across the connection's lifetime. `Connection::prepare` always re-parses. + --- ## `CREATE TABLE` @@ -90,7 +116,7 @@ These are full-citizen indexes — they're visible via `.tables`-adjacent catalo CREATE INDEX ON USING hnsw (); ``` -Builds an [HNSW](https://arxiv.org/abs/1603.09320) approximate-nearest-neighbor index over a `VECTOR(N)` column. The query optimizer recognizes `ORDER BY vec_distance_l2(col, literal) LIMIT k` (or the cosine / dot variants) on an HNSW-indexed column and probes the graph instead of full-scanning. +Builds an [HNSW](https://arxiv.org/abs/1603.09320) approximate-nearest-neighbor index over a `VECTOR(N)` column. The query optimizer recognizes `ORDER BY vec_distance_l2(col, literal) LIMIT k` (or the cosine / dot variants) on an HNSW-indexed column and probes the graph instead of full-scanning. SQLR-23 — the second arg can be either an inline `[...]` literal *or* a bound `Value::Vector(...)` parameter via `Statement::query_with_params`; the optimizer recognizes both, so prepared-statement KNN queries still take the graph shortcut. - Recall@10 ≥ 0.95 at default parameters (`M=16`, `ef_construction=200`, `ef_search=50`). Parameters aren't tunable from SQL yet — see Q2 of [`docs/phase-7-plan.md`](phase-7-plan.md). - The index is built incrementally on `INSERT`. `DELETE` / `UPDATE` mark the index `needs_rebuild`; the next save rebuilds from current rows. diff --git a/src/connection.rs b/src/connection.rs index 861507e..3eec9e2 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -1,4 +1,4 @@ -//! Public `Connection` / `Statement` / `Rows` / `Row` API (Phase 5a). +//! Public `Connection` / `Statement` / `Rows` / `Row` API (Phase 5a + SQLR-23). //! //! This is the stable surface external consumers bind against — Rust //! callers use it directly, language SDKs (Python, Node.js, Go) bind @@ -33,9 +33,28 @@ //! accessible via `sqlrite::sql::...` for the engine's own tests //! and for the desktop app — but those paths aren't considered //! stable API. +//! +//! # Prepared statements & parameter binding (SQLR-23) +//! +//! `Connection::prepare` parses the SQL once and stashes the AST on +//! the returned `Statement`. Subsequent calls to `Statement::query` / +//! `Statement::run` execute against the cached AST without re-running +//! sqlparser. Bound versions ([`Statement::query_with_params`] / +//! [`Statement::execute_with_params`]) accept a `&[Value]` slice that is +//! substituted into the cached AST at execute time — including +//! `Value::Vector(...)` for HNSW-eligible KNN queries, where binding +//! the query vector skips per-iter lexing of the 4 KB bracket-array +//! literal. +//! +//! [`Connection::prepare_cached`] adds a small per-connection LRU +//! (default cap 16) so a hot SQL string is parsed exactly once across +//! every call, not once per `prepare()`. Matches the rusqlite pattern. +use std::collections::VecDeque; use std::path::Path; +use std::sync::Arc; +use sqlparser::ast::Statement as AstStatement; use sqlparser::dialect::SQLiteDialect; use sqlparser::parser::Parser; @@ -44,8 +63,13 @@ use crate::sql::db::database::Database; use crate::sql::db::table::Value; use crate::sql::executor::execute_select_rows; use crate::sql::pager::{AccessMode, open_database_with_mode, save_database}; +use crate::sql::params::{rewrite_placeholders, substitute_params}; use crate::sql::parser::select::SelectQuery; -use crate::sql::process_command; +use crate::sql::process_ast_with_render; + +/// Default capacity of the per-connection prepared-statement plan cache. +/// Matches rusqlite's default; tweak with [`Connection::set_prepared_cache_capacity`]. +const DEFAULT_PREP_CACHE_CAP: usize = 16; /// A handle to a SQLRite database. Opens a file or an in-memory DB; /// drop it to close. Every mutating statement auto-saves (except inside @@ -68,6 +92,13 @@ use crate::sql::process_command; /// multi-threaded access. pub struct Connection { db: Database, + /// SQLR-23 — small SQL→cached-plan LRU. Keyed by the verbatim SQL + /// string the caller passed to `prepare_cached`. Stored as a + /// `VecDeque` rather than a HashMap+linked-list because the + /// expected capacity is small (default 16) — linear scan is fine + /// and the implementation stays dependency-free. + prep_cache: VecDeque<(String, Arc)>, + prep_cache_cap: usize, } impl Connection { @@ -97,7 +128,7 @@ impl Connection { save_database(&mut fresh, path)?; fresh }; - Ok(Self { db }) + Ok(Self::wrap(db)) } /// Opens an existing database file for read-only access. Takes a @@ -113,16 +144,22 @@ impl Connection { .unwrap_or("db") .to_string(); let db = open_database_with_mode(path, db_name, AccessMode::ReadOnly)?; - Ok(Self { db }) + Ok(Self::wrap(db)) } /// Opens a transient in-memory database. No file is touched and no /// locks are taken; state lives for the lifetime of the /// `Connection` and is discarded on drop. pub fn open_in_memory() -> Result { - Ok(Self { - db: Database::new("memdb".to_string()), - }) + Ok(Self::wrap(Database::new("memdb".to_string()))) + } + + fn wrap(db: Database) -> Self { + Self { + db, + prep_cache: VecDeque::new(), + prep_cache_cap: DEFAULT_PREP_CACHE_CAP, + } } /// Parses and executes one SQL statement. For DDL (`CREATE TABLE`, @@ -135,16 +172,67 @@ impl Connection { /// just returns the rendered status — use [`Connection::prepare`] /// and [`Statement::query`] to iterate typed rows. pub fn execute(&mut self, sql: &str) -> Result { - process_command(sql, &mut self.db) + crate::sql::process_command(sql, &mut self.db) } /// Prepares a statement for repeated execution or row iteration. - /// The SQL is parsed once and validated; the resulting - /// [`Statement`] can be executed multiple times. Today this is - /// primarily useful for SELECT (to reach the typed-row API); - /// parameter binding and prepared-plan caching are future work. + /// SQLR-23: the SQL is parsed once at prepare time (sqlparser walk + /// plus placeholder rewriting), and the resulting AST is cached + /// on the [`Statement`] for re-execution without further parsing. + /// + /// Use [`Statement::query`] / [`Statement::run`] for unbound + /// execution, or [`Statement::query_with_params`] / + /// [`Statement::execute_with_params`] to substitute `?` + /// placeholders. pub fn prepare<'c>(&'c mut self, sql: &str) -> Result> { - Statement::new(self, sql) + let plan = Arc::new(CachedPlan::compile(sql)?); + Ok(Statement { conn: self, plan }) + } + + /// Same as [`Connection::prepare`], but consults a small + /// per-connection LRU first. SQLR-23 — for hot statements + /// (the body of an INSERT loop, a frequently-rerun lookup) the + /// sqlparser walk is amortized to once across the connection's + /// lifetime, not once per `prepare()`. + /// + /// Default cache capacity is 16; tune with + /// [`Connection::set_prepared_cache_capacity`]. + pub fn prepare_cached<'c>(&'c mut self, sql: &str) -> Result> { + // Lookup-or-insert. Found entries are also moved to the back + // (most-recently-used) so capacity-eviction runs LRU. + let plan = if let Some(pos) = self.prep_cache.iter().position(|(k, _)| k == sql) { + let (k, v) = self.prep_cache.remove(pos).unwrap(); + self.prep_cache.push_back((k, Arc::clone(&v))); + v + } else { + let plan = Arc::new(CachedPlan::compile(sql)?); + self.prep_cache + .push_back((sql.to_string(), Arc::clone(&plan))); + while self.prep_cache.len() > self.prep_cache_cap { + self.prep_cache.pop_front(); + } + plan + }; + Ok(Statement { conn: self, plan }) + } + + /// SQLR-23 — sets the maximum number of cached prepared plans + /// (matches `prepare_cached`'s default 16). Reducing below the + /// current size evicts the oldest entries; setting to 0 disables + /// caching but `prepare_cached` still works (it just always + /// re-parses). + pub fn set_prepared_cache_capacity(&mut self, cap: usize) { + self.prep_cache_cap = cap; + while self.prep_cache.len() > cap { + self.prep_cache.pop_front(); + } + } + + /// SQLR-23 — current number of plans held by the prepared-statement + /// cache. Useful for tests / introspection; not load-bearing for + /// the public API. + pub fn prepared_cache_len(&self) -> usize { + self.prep_cache.len() } /// Returns `true` while a `BEGIN … COMMIT/ROLLBACK` block is open @@ -203,33 +291,80 @@ impl std::fmt::Debug for Connection { .field("in_transaction", &self.db.in_transaction()) .field("read_only", &self.db.is_read_only()) .field("tables", &self.db.tables.len()) + .field("prep_cache_len", &self.prep_cache.len()) .finish() } } -/// A prepared statement bound to a specific connection lifetime. -/// Today this is a thin wrapper around the raw SQL; Phase 5's cursor -/// work will grow it into a real prepared-plan cache. -pub struct Statement<'c> { - conn: &'c mut Connection, +/// SQLR-23 — the parse-once-execute-many representation. Built by +/// `CachedPlan::compile` (sqlparser walk + placeholder rewriting + +/// SELECT narrowing) and shared between every `Statement` that hits +/// the same SQL string in `prepare_cached`. +#[derive(Debug)] +struct CachedPlan { + /// Original SQL — kept for diagnostic output. + #[allow(dead_code)] sql: String, - kind: StatementKind, + /// AST after `?` → `?N` placeholder rewriting. Cloned per execute + /// so the substitution pass leaves the cached copy intact. + ast: AstStatement, + /// Total `?` placeholder count in the source SQL. Strict bind + /// validation in `query_with_params` / `execute_with_params` + /// uses this. + param_count: usize, + /// SELECT narrowing — cached so `query()` doesn't redo the + /// `SelectQuery::new` walk for unbound SELECTs. `None` for + /// non-SELECT statements. + select: Option, } -enum StatementKind { - Select(SelectQuery), - Other, +impl CachedPlan { + fn compile(sql: &str) -> Result { + let dialect = SQLiteDialect {}; + let mut ast = Parser::parse_sql(&dialect, sql).map_err(SQLRiteError::from)?; + let Some(mut stmt) = ast.pop() else { + return Err(SQLRiteError::General("no statement to prepare".to_string())); + }; + if !ast.is_empty() { + return Err(SQLRiteError::General( + "prepare() accepts a single statement; found more than one".to_string(), + )); + } + let param_count = rewrite_placeholders(&mut stmt); + let select = match &stmt { + AstStatement::Query(_) => Some(SelectQuery::new(&stmt)?), + _ => None, + }; + Ok(Self { + sql: sql.to_string(), + ast: stmt, + param_count, + select, + }) + } +} + +/// A prepared statement bound to a specific connection lifetime. +/// +/// SQLR-23 — `Statement` carries the parsed AST (parsed exactly once +/// at prepare time), not just the raw SQL. `query` / `run` execute +/// against the cached AST; `query_with_params` / `execute_with_params` +/// clone the AST and substitute `?` placeholders before dispatch. +pub struct Statement<'c> { + conn: &'c mut Connection, + plan: Arc, } impl std::fmt::Debug for Statement<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Statement") - .field("sql", &self.sql) + .field("sql", &self.plan.sql) + .field("param_count", &self.plan.param_count) .field( "kind", - &match self.kind { - StatementKind::Select(_) => "Select", - StatementKind::Other => "Other", + &match self.plan.select { + Some(_) => "Select", + None => "Other", }, ) .finish() @@ -237,66 +372,131 @@ impl std::fmt::Debug for Statement<'_> { } impl<'c> Statement<'c> { - fn new(conn: &'c mut Connection, sql: &str) -> Result { - // Parse once at prepare time so syntax errors surface early. - let dialect = SQLiteDialect {}; - let mut ast = Parser::parse_sql(&dialect, sql).map_err(SQLRiteError::from)?; - let Some(stmt) = ast.pop() else { - return Err(SQLRiteError::General("no statement to prepare".to_string())); - }; - if !ast.is_empty() { - return Err(SQLRiteError::General( - "prepare() accepts a single statement; found more than one".to_string(), - )); - } - let kind = match &stmt { - sqlparser::ast::Statement::Query(_) => StatementKind::Select(SelectQuery::new(&stmt)?), - _ => StatementKind::Other, - }; - Ok(Self { - conn, - sql: sql.to_string(), - kind, - }) + /// Number of `?` placeholders detected in the source SQL. Strict + /// arity validation: passing a slice of a different length to + /// `query_with_params` / `execute_with_params` returns a typed + /// error. + pub fn parameter_count(&self) -> usize { + self.plan.param_count } /// Executes a prepared non-query statement. Equivalent to /// [`Connection::execute`] — included for parity with the - /// typed-row `query()` so callers who want `Statement::run` / `Statement::query` - /// symmetry get it. + /// typed-row `query()` so callers who want `Statement::run` / + /// `Statement::query` symmetry get it. + /// + /// Errors if the prepared SQL contains `?` placeholders — use + /// [`Statement::execute_with_params`] for those. pub fn run(&mut self) -> Result { - self.conn.execute(&self.sql) + if self.plan.param_count > 0 { + return Err(SQLRiteError::General(format!( + "statement has {} `?` placeholder(s); call execute_with_params()", + self.plan.param_count + ))); + } + let ast = self.plan.ast.clone(); + process_ast_with_render(ast, &mut self.conn.db).map(|o| o.status) + } + + /// SQLR-23 — executes a prepared non-SELECT statement after binding + /// `?` placeholders to `params` (positional, in source order). + /// + /// Use this for parameterized INSERT / UPDATE / DELETE — the + /// substitution clones the cached AST, fills in the `?` slots + /// from `params`, and dispatches without re-running sqlparser. + /// For SELECT, prefer [`Statement::query_with_params`]. + pub fn execute_with_params(&mut self, params: &[Value]) -> Result { + self.check_arity(params)?; + let mut ast = self.plan.ast.clone(); + if !params.is_empty() { + substitute_params(&mut ast, params)?; + } + process_ast_with_render(ast, &mut self.conn.db).map(|o| o.status) } /// Runs a SELECT and returns a [`Rows`] iterator over typed rows. /// Errors if the prepared statement isn't a SELECT. + /// + /// SQLR-23 — uses the SELECT narrowing cached at prepare time; + /// no per-call sqlparser walk. Errors if the prepared SQL + /// contains `?` placeholders — use [`Statement::query_with_params`] + /// for those. pub fn query(&self) -> Result { - match &self.kind { - StatementKind::Select(sq) => { - let result = execute_select_rows(sq.clone(), &self.conn.db)?; - Ok(Rows { - columns: result.columns, - rows: result.rows.into_iter(), - }) - } - StatementKind::Other => Err(SQLRiteError::General( + if self.plan.param_count > 0 { + return Err(SQLRiteError::General(format!( + "statement has {} `?` placeholder(s); call query_with_params()", + self.plan.param_count + ))); + } + let Some(sq) = self.plan.select.as_ref() else { + return Err(SQLRiteError::General( "query() only works on SELECT statements; use run() for DDL/DML".to_string(), - )), + )); + }; + let result = execute_select_rows(sq.clone(), &self.conn.db)?; + Ok(Rows { + columns: result.columns, + rows: result.rows.into_iter(), + }) + } + + /// SQLR-23 — runs a SELECT and returns a [`Rows`] iterator after + /// binding `?` placeholders to `params`. Positional, source-order + /// indexing — `params[0]` is `?1`, `params[1]` is `?2`, etc. + /// + /// Vector parameters (`Value::Vector(...)`) substitute as the + /// in-band bracket-array shape the executor recognizes, so a + /// bound query vector still triggers the HNSW probe optimizer + /// (Phase 7d.2 KNN shortcut). + pub fn query_with_params(&self, params: &[Value]) -> Result { + self.check_arity(params)?; + if self.plan.select.is_none() { + return Err(SQLRiteError::General( + "query_with_params() only works on SELECT statements; use execute_with_params() \ + for DDL/DML" + .to_string(), + )); + } + // Re-narrow against the substituted AST. The narrow walk is + // cheap (it pulls projection/WHERE/ORDER BY into typed + // structs), and rerunning it ensures the substituted literals + // (e.g. a bracket-array vector) flow through `SelectQuery`. + let mut ast = self.plan.ast.clone(); + if !params.is_empty() { + substitute_params(&mut ast, params)?; + } + let sq = SelectQuery::new(&ast)?; + let result = execute_select_rows(sq, &self.conn.db)?; + Ok(Rows { + columns: result.columns, + rows: result.rows.into_iter(), + }) + } + + fn check_arity(&self, params: &[Value]) -> Result<()> { + if params.len() != self.plan.param_count { + return Err(SQLRiteError::General(format!( + "expected {} parameter{}, got {}", + self.plan.param_count, + if self.plan.param_count == 1 { "" } else { "s" }, + params.len() + ))); } + Ok(()) } /// Column names this statement will produce, in projection order. /// `None` for non-SELECT statements. pub fn column_names(&self) -> Option> { - match &self.kind { - StatementKind::Select(_) => { + match &self.plan.select { + Some(_) => { // We can't know the concrete column list without // running the query (it depends on the table schema // and the projection). Callers who need it up front // should call query() and inspect Rows::columns. None } - StatementKind::Other => None, + None => None, } } } @@ -701,4 +901,240 @@ mod tests { let err = row.get::(99).unwrap_err(); assert!(format!("{err}").contains("out of bounds")); } + + // ----------------------------------------------------------------- + // SQLR-23 — prepared-statement plan cache + parameter binding + // ----------------------------------------------------------------- + + #[test] + fn parameter_count_reflects_question_marks() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER, b TEXT);").unwrap(); + let stmt = conn.prepare("SELECT a, b FROM t WHERE a = ?").unwrap(); + assert_eq!(stmt.parameter_count(), 1); + let stmt = conn + .prepare("SELECT a, b FROM t WHERE a = ? AND b = ?") + .unwrap(); + assert_eq!(stmt.parameter_count(), 2); + let stmt = conn.prepare("SELECT a FROM t").unwrap(); + assert_eq!(stmt.parameter_count(), 0); + } + + #[test] + fn query_with_params_binds_scalars() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER PRIMARY KEY, b TEXT);") + .unwrap(); + conn.execute("INSERT INTO t (a, b) VALUES (1, 'alice');") + .unwrap(); + conn.execute("INSERT INTO t (a, b) VALUES (2, 'bob');") + .unwrap(); + conn.execute("INSERT INTO t (a, b) VALUES (3, 'carol');") + .unwrap(); + + let stmt = conn.prepare("SELECT b FROM t WHERE a = ?").unwrap(); + let rows = stmt + .query_with_params(&[Value::Integer(2)]) + .unwrap() + .collect_all() + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].get::(0).unwrap(), "bob"); + } + + #[test] + fn execute_with_params_binds_insert_values() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER, b TEXT);").unwrap(); + + let mut stmt = conn.prepare("INSERT INTO t (a, b) VALUES (?, ?)").unwrap(); + stmt.execute_with_params(&[Value::Integer(7), Value::Text("hi".into())]) + .unwrap(); + stmt.execute_with_params(&[Value::Integer(8), Value::Text("yo".into())]) + .unwrap(); + + let stmt = conn.prepare("SELECT a, b FROM t").unwrap(); + let rows = stmt.query().unwrap().collect_all().unwrap(); + assert_eq!(rows.len(), 2); + assert!( + rows.iter() + .any(|r| r.get::(0).unwrap() == 7 && r.get::(1).unwrap() == "hi") + ); + assert!( + rows.iter() + .any(|r| r.get::(0).unwrap() == 8 && r.get::(1).unwrap() == "yo") + ); + } + + #[test] + fn arity_mismatch_returns_clean_error() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER, b TEXT);").unwrap(); + let stmt = conn + .prepare("SELECT * FROM t WHERE a = ? AND b = ?") + .unwrap(); + let err = stmt.query_with_params(&[Value::Integer(1)]).unwrap_err(); + assert!(format!("{err}").contains("expected 2 parameter")); + } + + #[test] + fn run_and_query_reject_when_placeholders_present() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER);").unwrap(); + let mut stmt_select = conn.prepare("SELECT a FROM t WHERE a = ?").unwrap(); + let err = stmt_select.query().unwrap_err(); + assert!(format!("{err}").contains("query_with_params")); + let err = stmt_select.run().unwrap_err(); + assert!(format!("{err}").contains("execute_with_params")); + } + + #[test] + fn null_param_compares_against_null() { + // a = NULL is *false* in SQL three-valued logic; binding NULL + // must match SQLite's behavior so callers can rely on the same + // semantics. + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER);").unwrap(); + conn.execute("INSERT INTO t (a) VALUES (1);").unwrap(); + let stmt = conn.prepare("SELECT a FROM t WHERE a = ?").unwrap(); + let rows = stmt + .query_with_params(&[Value::Null]) + .unwrap() + .collect_all() + .unwrap(); + assert_eq!(rows.len(), 0); + } + + #[test] + fn vector_param_substitutes_through_select() { + // Non-HNSW path: a small VECTOR table + brute-force ORDER BY + // exercises the substitution into the ORDER BY expression + // and the bracket-array shape eval_expr_scope expects. + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE v (id INTEGER PRIMARY KEY, e VECTOR(3));") + .unwrap(); + conn.execute("INSERT INTO v (id, e) VALUES (1, [1.0, 0.0, 0.0]);") + .unwrap(); + conn.execute("INSERT INTO v (id, e) VALUES (2, [0.0, 1.0, 0.0]);") + .unwrap(); + conn.execute("INSERT INTO v (id, e) VALUES (3, [0.0, 0.0, 1.0]);") + .unwrap(); + + let stmt = conn + .prepare("SELECT id FROM v ORDER BY vec_distance_l2(e, ?) ASC LIMIT 1") + .unwrap(); + let rows = stmt + .query_with_params(&[Value::Vector(vec![1.0, 0.0, 0.0])]) + .unwrap() + .collect_all() + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].get::(0).unwrap(), 1); + } + + #[test] + fn prepare_cached_reuses_plans() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER);").unwrap(); + for n in 1..=3 { + conn.execute(&format!("INSERT INTO t (a) VALUES ({n});")) + .unwrap(); + } + + // First call populates the cache; second hits the same entry. + let _ = conn.prepare_cached("SELECT a FROM t WHERE a = ?").unwrap(); + let _ = conn.prepare_cached("SELECT a FROM t WHERE a = ?").unwrap(); + assert_eq!(conn.prepared_cache_len(), 1); + + // Distinct SQL widens the cache. + let _ = conn.prepare_cached("SELECT a FROM t").unwrap(); + assert_eq!(conn.prepared_cache_len(), 2); + } + + #[test] + fn prepare_cached_evicts_when_over_capacity() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER);").unwrap(); + conn.set_prepared_cache_capacity(2); + let _ = conn.prepare_cached("SELECT a FROM t").unwrap(); + let _ = conn.prepare_cached("SELECT a FROM t WHERE a = ?").unwrap(); + assert_eq!(conn.prepared_cache_len(), 2); + // Third distinct SQL evicts the oldest entry (the FROM-only SELECT). + let _ = conn.prepare_cached("SELECT a FROM t WHERE a > ?").unwrap(); + assert_eq!(conn.prepared_cache_len(), 2); + } + + /// SQLR-23 — the headline VECTOR-binding case. With an HNSW index + /// attached, the optimizer hook recognizes + /// `ORDER BY vec_distance_l2(col, ?) LIMIT k` even when the second + /// arg is a bound parameter, because substitution lowers + /// `Value::Vector` into the same bracket-array shape an inline + /// `[…]` literal produces. Self-query: querying for one of the + /// corpus's own vectors must return that vector as the nearest. + #[test] + fn vector_bind_through_hnsw_optimizer() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE v (id INTEGER PRIMARY KEY, e VECTOR(4));") + .unwrap(); + let corpus: [(i64, [f32; 4]); 5] = [ + (1, [1.0, 0.0, 0.0, 0.0]), + (2, [0.0, 1.0, 0.0, 0.0]), + (3, [0.0, 0.0, 1.0, 0.0]), + (4, [0.0, 0.0, 0.0, 1.0]), + (5, [0.5, 0.5, 0.5, 0.5]), + ]; + for (id, vec) in corpus { + conn.execute(&format!( + "INSERT INTO v (id, e) VALUES ({id}, [{}, {}, {}, {}]);", + vec[0], vec[1], vec[2], vec[3] + )) + .unwrap(); + } + conn.execute("CREATE INDEX v_hnsw ON v USING hnsw (e);") + .unwrap(); + + let stmt = conn + .prepare("SELECT id FROM v ORDER BY vec_distance_l2(e, ?) ASC LIMIT 1") + .unwrap(); + // Query with id=3's vector — expect id=3 back. + let rows = stmt + .query_with_params(&[Value::Vector(vec![0.0, 0.0, 1.0, 0.0])]) + .unwrap() + .collect_all() + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].get::(0).unwrap(), 3); + + // Query with id=1's vector — expect id=1. + let rows = stmt + .query_with_params(&[Value::Vector(vec![1.0, 0.0, 0.0, 0.0])]) + .unwrap() + .collect_all() + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].get::(0).unwrap(), 1); + } + + #[test] + fn prepare_cached_executes_the_same_as_prepare() { + let mut conn = Connection::open_in_memory().unwrap(); + conn.execute("CREATE TABLE t (a INTEGER PRIMARY KEY, b TEXT);") + .unwrap(); + let mut ins = conn + .prepare_cached("INSERT INTO t (a, b) VALUES (?, ?)") + .unwrap(); + ins.execute_with_params(&[Value::Integer(1), Value::Text("alpha".into())]) + .unwrap(); + ins.execute_with_params(&[Value::Integer(2), Value::Text("beta".into())]) + .unwrap(); + + let stmt = conn.prepare_cached("SELECT b FROM t WHERE a = ?").unwrap(); + let rows = stmt + .query_with_params(&[Value::Integer(2)]) + .unwrap() + .collect_all() + .unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].get::(0).unwrap(), "beta"); + } } diff --git a/src/lib.rs b/src/lib.rs index 65b76d9..9edcd52 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,7 +88,9 @@ pub use sql::pager::{ AccessMode, MASTER_TABLE_NAME, open_database, open_database_read_only, open_database_with_mode, save_database, }; -pub use sql::{CommandOutput, process_command, process_command_with_render}; +pub use sql::{ + CommandOutput, process_ast_with_render, process_command, process_command_with_render, +}; // Re-export sqlparser so downstream crates (the Tauri desktop app, the // eventual WASM bindings) can reach into the AST without pulling a diff --git a/src/sql/mod.rs b/src/sql/mod.rs index c4b660a..29688d1 100644 --- a/src/sql/mod.rs +++ b/src/sql/mod.rs @@ -4,6 +4,7 @@ pub mod executor; pub mod fts; pub mod hnsw; pub mod pager; +pub mod params; pub mod parser; // pub mod tokenizer; @@ -87,8 +88,6 @@ pub fn process_command(query: &str, db: &mut Database) -> Result { /// from the returned struct. pub fn process_command_with_render(query: &str, db: &mut Database) -> Result { let dialect = SQLiteDialect {}; - let message: String; - let mut rendered: Option = None; let mut ast = Parser::parse_sql(&dialect, query).map_err(SQLRiteError::from)?; if ast.len() > 1 { @@ -107,6 +106,16 @@ pub fn process_command_with_render(query: &str, db: &mut Database) -> Result Result { + let message: String; + let mut rendered: Option = None; // Transaction boundary statements are routed to Database-level // handlers before we even inspect the rest of the AST. They don't diff --git a/src/sql/params.rs b/src/sql/params.rs new file mode 100644 index 0000000..0b9ad23 --- /dev/null +++ b/src/sql/params.rs @@ -0,0 +1,271 @@ +//! Prepared-statement parameter binding (SQLR-23). +//! +//! Two responsibilities: +//! +//! 1. **Placeholder rewriting at prepare time.** The user writes `?` in +//! the SQL; sqlparser parses each as `Expr::Value(Placeholder("?"))`. +//! We walk the parsed AST left-to-right and rewrite each bare `?` to +//! `?N` (1-indexed source order) so the later substitution pass knows +//! which slot to bind. The rewritten AST is what `Statement` caches. +//! +//! 2. **Substitution at execute time.** Given the cached AST and a +//! `&[Value]` slice, walk a clone of the AST and replace every +//! `Expr::Value(Placeholder("?N"))` with the matching `params[N-1]`. +//! +//! Substitution lowers the bound value into a node shape the rest of the +//! pipeline already understands: +//! +//! - Scalars (`Integer`, `Real`, `Text`, `Bool`, `Null`) become +//! `Expr::Value(...)` literals — same shape an inline literal would +//! parse to. Existing executor / parser arms handle them unchanged. +//! - Vectors become `Expr::Identifier { quote_style: Some('['), value: "" }`, +//! which is the in-band form sqlparser produces for inline bracket-array +//! literals like `[0.1, 0.2, ...]`. The INSERT parser, the executor's +//! `eval_expr_scope`, and the HNSW probe optimizer all already recognize +//! that shape, so a bound `Value::Vector(...)` flows through every path +//! that an inline `[...]` literal does — including the HNSW shortcut. +//! +//! Doing it as an AST-rewrite (rather than threading `&[Value]` through +//! the executor) keeps the diff focused: every existing executor arm +//! sees concrete literals, exactly as it does today on inline-params SQL. + +use std::ops::ControlFlow; + +use sqlparser::ast::{ + Expr, Ident, Statement, Value as AstValue, ValueWithSpan, visit_expressions_mut, +}; +use sqlparser::tokenizer::Span; + +use crate::error::{Result, SQLRiteError}; +use crate::sql::db::table::Value; + +/// Walks every expression in `stmt` and rewrites bare `?` placeholders to +/// `?N` (1-indexed source order). Returns the total parameter count. +/// +/// Idempotent for already-numbered placeholders: `?1`, `?2`, … pass +/// through unchanged. We deliberately don't try to *renumber* already- +/// numbered placeholders — that's a foot-gun (the user might use the +/// same index twice on purpose to bind once and reference twice), and +/// `Statement::new` runs this exactly once on a freshly-parsed AST. +pub fn rewrite_placeholders(stmt: &mut Statement) -> usize { + let mut counter: usize = 0; + let _ = visit_expressions_mut(stmt, |expr| { + if let Expr::Value(v) = expr + && let AstValue::Placeholder(s) = &mut v.value + && s == "?" + { + counter += 1; + *s = format!("?{counter}"); + } + ControlFlow::<()>::Continue(()) + }); + counter +} + +/// Substitutes every `?N` placeholder in `stmt` with the matching value +/// from `params`. Mutates the AST in place — callers should clone first +/// if they want the original back. +/// +/// Errors if the AST references a placeholder index outside `params`, +/// or if a non-canonical placeholder form (`:name`, `$1`) is encountered. +pub fn substitute_params(stmt: &mut Statement, params: &[Value]) -> Result<()> { + let mut bind_err: Option = None; + let _ = visit_expressions_mut(stmt, |expr| { + let Expr::Value(v) = expr else { + return ControlFlow::Continue(()); + }; + let placeholder_str = match &v.value { + AstValue::Placeholder(s) => s.clone(), + _ => return ControlFlow::Continue(()), + }; + let idx = match placeholder_index(&placeholder_str) { + Some(i) => i, + None => { + bind_err = Some(SQLRiteError::NotImplemented(format!( + "unsupported placeholder form `{placeholder_str}`; only `?` and `?N` are supported" + ))); + return ControlFlow::Break(()); + } + }; + let Some(value) = params.get(idx) else { + bind_err = Some(SQLRiteError::General(format!( + "missing bind value for `?{}` (got {} parameter{})", + idx + 1, + params.len(), + if params.len() == 1 { "" } else { "s" } + ))); + return ControlFlow::Break(()); + }; + *expr = value_to_expr(value); + ControlFlow::<()>::Continue(()) + }); + if let Some(e) = bind_err { + return Err(e); + } + Ok(()) +} + +/// Decode a `Placeholder("?N")` string into its 0-indexed slot. Returns +/// `None` for any non-canonical form (`:name`, `$1`, bare `?` after +/// rewriting — that last case shouldn't happen but is rejected +/// defensively). +fn placeholder_index(s: &str) -> Option { + let n = s.strip_prefix('?')?.parse::().ok()?; + if n == 0 { + return None; + } + Some(n - 1) +} + +/// Build the AST `Expr` equivalent of a runtime `Value`. The shapes +/// match what `sqlparser` produces for inline literals so downstream +/// executor code paths don't need to change. +fn value_to_expr(v: &Value) -> Expr { + match v { + Value::Null => Expr::Value(ValueWithSpan { + value: AstValue::Null, + span: Span::empty(), + }), + Value::Integer(i) => Expr::Value(ValueWithSpan { + value: AstValue::Number(i.to_string(), false), + span: Span::empty(), + }), + Value::Real(f) => Expr::Value(ValueWithSpan { + // f64::Display picks the shortest round-tripping form; + // re-parsing it back via str::parse:: is exact. + value: AstValue::Number(f.to_string(), false), + span: Span::empty(), + }), + Value::Text(s) => Expr::Value(ValueWithSpan { + value: AstValue::SingleQuotedString(s.clone()), + span: Span::empty(), + }), + Value::Bool(b) => Expr::Value(ValueWithSpan { + value: AstValue::Boolean(*b), + span: Span::empty(), + }), + Value::Vector(v) => { + // Inline bracket-array form. `i.value` carries the inner + // CSV without brackets — `format!("[{}]", i.value)` at the + // consumer side reconstructs the literal that + // `parse_vector_literal` accepts. + let inner = format_vector_inner(v); + Expr::Identifier(Ident { + value: inner, + quote_style: Some('['), + span: Span::empty(), + }) + } + } +} + +fn format_vector_inner(v: &[f32]) -> String { + // Preallocate generously: each f32 averages ~8 chars + ", ". + let mut s = String::with_capacity(v.len() * 10); + for (i, x) in v.iter().enumerate() { + if i > 0 { + s.push_str(", "); + } + s.push_str(&x.to_string()); + } + s +} + +#[cfg(test)] +mod tests { + use super::*; + use sqlparser::dialect::SQLiteDialect; + use sqlparser::parser::Parser; + + fn parse_one(sql: &str) -> Statement { + let mut ast = Parser::parse_sql(&SQLiteDialect {}, sql).unwrap(); + ast.pop().unwrap() + } + + #[test] + fn rewrite_assigns_indices_in_source_order() { + let mut stmt = parse_one("SELECT * FROM t WHERE a = ? AND b = ? AND c = ?"); + let n = rewrite_placeholders(&mut stmt); + assert_eq!(n, 3); + let sql = stmt.to_string(); + assert!(sql.contains("?1")); + assert!(sql.contains("?2")); + assert!(sql.contains("?3")); + } + + #[test] + fn rewrite_zero_for_no_placeholders() { + let mut stmt = parse_one("SELECT * FROM t WHERE a = 1"); + assert_eq!(rewrite_placeholders(&mut stmt), 0); + } + + #[test] + fn rewrite_idempotent_on_numbered_placeholders() { + // `?1` parses with placeholder string `?1`. Walking again must + // not double-number. + let mut stmt = parse_one("SELECT * FROM t WHERE a = ?1 AND b = ?2"); + let n = rewrite_placeholders(&mut stmt); + // Bare `?` count is zero — the existing `?1`/`?2` are left + // alone. The total parameter count is therefore reported as 0 + // here; callers using `?N` form should already know their + // arity from the source SQL. + assert_eq!(n, 0); + } + + #[test] + fn substitute_replaces_scalar_params() { + let mut stmt = parse_one("SELECT * FROM t WHERE a = ? AND b = ? AND c = ?"); + rewrite_placeholders(&mut stmt); + substitute_params( + &mut stmt, + &[ + Value::Integer(1), + Value::Text("x".into()), + Value::Bool(true), + ], + ) + .unwrap(); + let sql = stmt.to_string(); + assert!(sql.contains("a = 1"), "got: {sql}"); + assert!(sql.contains("b = 'x'"), "got: {sql}"); + // sqlparser renders Boolean::true as `true`. + assert!(sql.contains("c = true"), "got: {sql}"); + } + + #[test] + fn substitute_replaces_vector_param_as_bracket_array() { + let mut stmt = parse_one("SELECT id FROM t ORDER BY vec_distance_l2(v, ?) LIMIT 5"); + rewrite_placeholders(&mut stmt); + substitute_params(&mut stmt, &[Value::Vector(vec![0.1, 0.2, 0.3])]).unwrap(); + let sql = stmt.to_string(); + // sqlparser renders bracket-quoted Identifier as `[]`. + assert!(sql.contains("[0.1, 0.2, 0.3]"), "got: {sql}"); + } + + #[test] + fn substitute_errors_on_too_few_params() { + let mut stmt = parse_one("SELECT * FROM t WHERE a = ? AND b = ?"); + rewrite_placeholders(&mut stmt); + let err = substitute_params(&mut stmt, &[Value::Integer(1)]).unwrap_err(); + assert!(format!("{err}").contains("missing bind value")); + } + + #[test] + fn substitute_replaces_null_param() { + let mut stmt = parse_one("SELECT * FROM t WHERE a = ?"); + rewrite_placeholders(&mut stmt); + substitute_params(&mut stmt, &[Value::Null]).unwrap(); + let sql = stmt.to_string(); + assert!(sql.to_uppercase().contains("NULL"), "got: {sql}"); + } + + #[test] + fn placeholder_index_decodes_canonical_form() { + assert_eq!(placeholder_index("?1"), Some(0)); + assert_eq!(placeholder_index("?42"), Some(41)); + assert_eq!(placeholder_index("?"), None); + assert_eq!(placeholder_index("?0"), None); + assert_eq!(placeholder_index(":name"), None); + assert_eq!(placeholder_index("$1"), None); + } +}