feat(engine): prepared-statement plan cache + parameter binding (SQLR-23)#108
Closed
joaoh82 wants to merge 1 commit into
Closed
feat(engine): prepared-statement plan cache + parameter binding (SQLR-23)#108joaoh82 wants to merge 1 commit into
joaoh82 wants to merge 1 commit into
Conversation
…-23) `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<f32>)` 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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes SQLR-23 (highest-leverage perf task surfaced by the bench suite).
Connection::prepare_cached(sql)— per-connection LRU plan cache (default cap 16, matches rusqlite).set_prepared_cache_capacity(n)to tune.Statement::query_with_params(&[Value])/Statement::execute_with_params(&[Value])— bind?placeholders positionally. Strict arity check; clear typed errors on mismatch.Statementnow caches the parsed AST at prepare time.query()/run()/ the bound versions all dispatch through it without re-running sqlparser.Value::Vector(Vec<f32>)is a first-class bind type — a bound vector substitutes as the in-band bracket-array shapeeval_expr_scopeandtry_hnsw_probealready recognize. HNSW shortcut still fires on prepared queries.Approach: AST-rewrite. New module
src/sql/params.rs(a) renumbers bare?→?Nat prepare time and (b) clones + substitutes the cached AST at execute time. Zero changes to the executor / parser narrowing — every existing arm sees concrete literals exactly as it does today on inline-params SQL. Enabled thevisitorfeature onsqlparserto usevisit_expressions_mut.Bench harness flip + version bump
The SQLRite driver was flipped from per-call
inline_paramsSQL formatting toprepare_cached+ the bound API. Every workload'sWorkloadId.versionbumpedv1 → v2in lockstep so the methodology change is captured explicitly — old v1 envelopes stay readable; the comparison script flags cross-version pairs.Smoke results vs v1 baseline
Laptop smoke (read directionally only — pinned-host republish is SQLR-25):
Pre-existing limitation surfaced (not introduced)
W10 HNSW didn't widen its gap vs brute-force — but not for a SQLR-23 reason.
try_hnsw_probeis L2-only per its own docstring; W10's hot loop usesvec_distance_cosine, so the HNSW variant has been silently brute-forcing the entire time. SQLR-28 tracks widening the probe hook to cosine + dot, gating the proper W10 win.Tests
src/connection.rs: parameter count, scalar bind, vector bind via brute-force, HNSW + bound vector self-query (verifies optimizer still fires), arity mismatch, NULL three-valued logic, INSERT-loop binding,prepare_cachedreuse + LRU eviction.src/sql/params.rscovering placeholder rewriting, scalar / vector / null substitution, arity errors.All 475 lib tests + 20 MCP + 8 FFI green.
Test plan
cargo fmt --all -- --checkcargo build --workspace --exclude sqlrite-{desktop,python,nodejs,benchmarks} --all-targetscargo test --workspace --exclude sqlrite-{desktop,python,nodejs,benchmarks}— all greencargo clippy --workspace …— no new warnings on touched codecargo doc --workspace … --no-deps— cleancargo build -p sqlrite-benchmarks --features duckdbmake benchsmoke — all 12 workloads completed; v2 wins visible on parser-bound rowsFollow-ups (filed in marvinapp)
LIMIT ?🤖 Generated with Claude Code