chore: sqlite framework#2265
Conversation
31dc8cc to
a954f57
Compare
kkovaacs
left a comment
There was a problem hiding this comment.
Apart from the single nit I've added I think this looks good!
Thanks!
| //! (so an index on the column can be used): | ||
| //! | ||
| //! ```sql | ||
| //! ... WHERE col IN (SELECT value FROM rarray(?1)) |
There was a problem hiding this comment.
Yeah super cool!
| //! (so an index on the column can be used): | ||
| //! | ||
| //! ```sql | ||
| //! ... WHERE col IN (SELECT value FROM rarray(?1)) |
There was a problem hiding this comment.
Yeah super cool!
| "INSERT INTO validated_transactions \ | ||
| (id, block_num, account_id, account_patch, input_notes, output_notes, \ | ||
| initial_account_hash, final_account_hash) \ | ||
| VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8) \ | ||
| ON CONFLICT DO NOTHING", |
There was a problem hiding this comment.
I think we should consider moving the sql into dedicated .sql files.
This allows for:
- SQL syntax highlighting and LSP
- linting
- formatting
- not fighting the rust indentations
This is slightly less relevant here, since we have so few; but I'm thinking a structure like:
bin/validator/src/db/queries
/insert_transaction
mod.rs
tests.rs
insert_transaction.sql
/transaction_exists
mod.rs
tests.rs
transaction_exists.sql
/...
Doesn't need to be part of this PR though.
| fn in_memory() -> Connection { | ||
| let conn = Connection::open_in_memory().expect("open in-memory db"); | ||
| // `rarray()` is provided by rusqlite's `array` extension, which must be loaded per | ||
| // connection (the pool does this in `configure_connection`). | ||
| rusqlite::vtab::array::load_module(&conn).expect("load array module"); | ||
| conn.execute_batch( | ||
| "CREATE TABLE items (id INTEGER PRIMARY KEY, payload BLOB, label TEXT);", | ||
| ) | ||
| .expect("create table"); | ||
| conn | ||
| } |
There was a problem hiding this comment.
I would caution against this; there are some edge cases here that are non-obvious. See my impl from previous job.
Instead, I would suggest using tmpdir/tmpfile to create a concrete but temporary db.
| path: PathBuf, | ||
| /// When set, connections are configured `PRAGMA query_only = ON` and skip the writer-only | ||
| /// `journal_mode` setup — used for the reader pool. | ||
| query_only: bool, |
There was a problem hiding this comment.
Nit: read_only seems like a better name to me.
closes #2248
closes #2249
closes #1465
Summary
Introduces a light SQLite framework in crates/db (
miden-node-db) and migrates the validator onto it, removing Diesel. It handles a connection pool, type-enforced read/write transactions, always-cached queries, a type codec, and cacheable IN-list helpers.handle:
read(...)gives you aReadTx. It begins a read-only transaction that is never committed, sonothing it does can persist.
write(...)gives you aWriteTx. It begins a write transaction.query_rows,query_opt,query_one,exists,countandexecute(write only). Every one of them prepares its statement through SQLite's cache.The usage is like this:
Changelog