feat(query): prepared-statement plan cache, v1 (validation-skip only) - #216
Merged
Conversation
Adds a real prepared-statement handle, Database::prepare/execute_prepared_plan (and the Transaction twin), that skips semantic validation on repeat calls when it's provably safe to: a parameter's coarse category (null/scalar/list/map) determines validate_statement's outcome (Int vs String never does, both type Kind::Scalar), so a call whose every parameter matches a previously-validated fingerprint, at the same schema generation, can skip re-validating. Query planning (build_match_plan/apply_index_seeks) still reruns every call in this version -- a full plan cache needs to correlate a cached plan's embedded IndexSeek value back to the specific $param that produced it, and value-equality correlation isn't sound (two different params can coincidentally share a value, misattributing which one a cached literal should track). ParamSite/PathStep (params.rs) and IndexSeekOutcome/apply_index_seeks_tracked (planner.rs) are in place as forward-compatible infrastructure for that follow-up, once it has a sound provenance mechanism, but aren't wired into execution yet. GraphStore::schema_generation (marsdb-graph) invalidates the validation cache when a new index is declared, since that can change what a statement's validation-independent behavior... [no, indexes don't affect validation, only planning -- generation exists for the planning cache this version doesn't have yet, kept for forward compatibility and because Database::execute_prepared_plan already threads it through]. Executor gains _trusted variants of its two guarded entry points (execute_with_options_trusted, execute_in_write_transaction_with_options_trusted) that skip validate_statement; every existing call site is unchanged and still validates unconditionally. Full workspace test suite, clippy (default + arrow feature), fmt, and the openCypher TCK (3880/3880, unchanged) all pass. Benchmarked against the LDBC-style traversal workload this was motivated by: no measurable improvement (validation was not, in fact, the dominant per-call cost for these queries -- planning and/or the per-call AST clone and transaction-open overhead are). Landing this as a correct, tested increment; closing the actual gap needs either the deferred plan-caching work or a separate look at what's actually dominant.
New public API from the prepared-plan-cache work needs its own usage example alongside the existing execute/execute_with_options/ begin_transaction ones.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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
3 participants
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.
Why
MarsDB's existing "prepared statement" API only meant the caller had
pre-parsed the Cypher text once — every execution still ran full
semantic validation from scratch. Found while building an apples-to-
apples benchmark against LatticeDB and SQLite earlier this session:
SQLite's
sqlite3_prepare_v2genuinely compiles once and reuses thatacross calls, MarsDB didn't have an equivalent.
Worth saying plainly: the comparison that motivated this turned out to
be apples-to-oranges. LatticeDB bypasses a query language entirely for
its benchmark path, so it was never going to be a fair fight, and a
follow-up phase-breakdown diagnostic (on the same 10K-node graph)
measured clone+substitute+validate+plan at only ~17% of a 1-hop
query's total per-call time — the other ~83% is row evaluation, which
this doesn't touch. This PR is not a fix for that benchmark gap. It's
still a real, correct feature worth having on its own: a genuine
prepared-statement handle that pre-parses once and safely skips
re-validation on repeat calls.
What
Database::prepare(cypher) -> PreparedPlanparses once. Pass thehandle to
Database::execute_prepared_plan(&prepared, ¶ms, &options)(and the matching
Transactiontwin) to run it any number of timeswith different
params. Each call substitutes params into a freshclone of the stored AST as before; the new part is
PreparedPlan::can_skip_validation, which tracks a coarse per-paramnull/scalar/list/map fingerprint plus the last-seen
GraphStore::schema_generation()(new: anAtomicU64bumped wheneveran index is declared — indexes are never dropped, so a stale
generation can only mean "might be missing an index," never wrong
results). When a call's fingerprint and generation match a previously-
validated call,
semantic::validate_statementis skipped via a newexecute_*_with_options_trustedentry point onExecutor; every otherpath still validates. Query planning reruns every call — this v1 is
validation-skip only, not a plan cache in the fuller sense the name
might suggest.
Also drops the temporary phase-breakdown diagnostic test that was on
this branch during development (same treatment as the diagnostics used
while investigating #214 and #215 — profiling aids, not permanent
regression tests) and adds a plain README example for the new API next
to the existing
execute/execute_with_options/begin_transactionones.
Explicitly not in this PR
"skip planning too" design this started from) — v1 only skips
validation, planning still reruns every call.
marsdb-capimigration toPreparedPlan— capi's existingprepare/bind/execute path is untouched.
applicable since v1 doesn't cache plans at all yet.
Testing
cargo fmt --all -- --check,cargo clippy --workspace --all-targets -- -D warnings,cargo clippy -p marsdb -p marsdb-capi --all-targets --features arrow -- -D warnings,cargo test --workspace,cargo test -p marsdb -p marsdb-capi --features arrow,cargo check --manifest-path marsdb-python/Cargo.tomlall pass.cargo run --release -p marsdb-tck: 3880/3880, unchanged.marsdb-capiprepare_bind_execute_reusetest and the newmarsdb/tests/prepared_plan.rs(category-change correctness,schema-generation invalidation, MERGE exclusion) all pass.