sqlite: add stmt persistent flag - #62757
Conversation
|
Review requested:
|
There was a problem hiding this comment.
Pull request overview
Adds a new persistent option to DatabaseSync#prepare() in node:sqlite, exposing SQLite’s SQLITE_PREPARE_PERSISTENT hint to influence statement memory-allocation strategy for frequently reused prepared statements.
Changes:
- Add
options.persistentparsing/validation toDatabaseSync::Prepare()and passSQLITE_PREPARE_PERSISTENTviasqlite3_prepare_v3(). - Add parallel tests covering
persistent: true/false, type validation, and interoperability with other options. - Document the new option and update the implementation reference from
sqlite3_prepare_v2()tosqlite3_prepare_v3().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/node_sqlite.cc |
Parses options.persistent and uses sqlite3_prepare_v3(..., SQLITE_PREPARE_PERSISTENT) when enabled. |
test/parallel/test-sqlite-statement-sync.js |
Adds coverage for correct execution and argument validation for persistent. |
doc/api/sqlite.md |
Documents persistent and updates the underlying SQLite API reference/link definitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
504bd95 to
4f2ef78
Compare
|
Interesting thing: SQLITE_PREPARE_PERSISTENT is enabled by default on better-sqlite3 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62757 +/- ##
==========================================
+ Coverage 90.31% 90.33% +0.01%
==========================================
Files 751 751
Lines 249776 249795 +19
Branches 47173 47176 +3
==========================================
+ Hits 225587 225654 +67
+ Misses 15585 15531 -54
- Partials 8604 8610 +6
🚀 New features to boost your workflow:
|
|
At least makes sense to enable it by default for SQLTagStore statements I'd say. |
|
@nodejs/sqlite Friendly bump |
125d667 to
69fc714
Compare
|
@araujogui Can you please rebase from main? |
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
69fc714 to
d32e5e6
Compare
Rebased |
| database options or `true`. | ||
| * `allowUnknownNamedParameters` {boolean} If `true`, unknown named parameters | ||
| are ignored. **Default:** inherited from database options or `false`. | ||
| * `persistent` {boolean} If `true`, hints to SQLite that this statement will |
There was a problem hiding this comment.
Please add a history entry for this new public option; otherwise the docs imply it has been available since v22.5.0.
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/65157
description: Throw `ERR_INVALID_ARG_VALUE` if `sql` contains no statements.
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/62757
+ description: Add the `persistent` option.| * `persistent` {boolean} If `true`, hints to SQLite that this statement will | ||
| be reused many times, causing it to use a different memory allocation | ||
| strategy that reduces heap fragmentation. Corresponds to the | ||
| [`SQLITE_PREPARE_PERSISTENT`][] flag. **Default:** `false`. |
There was a problem hiding this comment.
SQLite does not guarantee reduced heap fragmentation; its documented current behavior is to avoid depleting lookaside memory.
| * `persistent` {boolean} If `true`, hints to SQLite that this statement will | |
| be reused many times, causing it to use a different memory allocation | |
| strategy that reduces heap fragmentation. Corresponds to the | |
| [`SQLITE_PREPARE_PERSISTENT`][] flag. **Default:** `false`. | |
| * `persistent` {boolean} If `true`, hints to SQLite that this statement will | |
| be retained for a long time and likely reused many times. SQLite currently | |
| responds to this hint by avoiding lookaside memory. Corresponds to the | |
| [`SQLITE_PREPARE_PERSISTENT`][] flag. **Default:** `false`. |
Add statement's persistent argument flag
Reference:
https://sqlite.org/c3ref/c_prepare_dont_log.html#sqlitepreparepersistent