Skip to content

sqlite: add stmt persistent flag - #62757

Open
araujogui wants to merge 4 commits into
nodejs:mainfrom
araujogui:sqlite-prepare-persistent
Open

sqlite: add stmt persistent flag#62757
araujogui wants to merge 4 commits into
nodejs:mainfrom
araujogui:sqlite-prepare-persistent

Conversation

@araujogui

Copy link
Copy Markdown
Member

Add statement's persistent argument flag

Reference:
https://sqlite.org/c3ref/c_prepare_dont_log.html#sqlitepreparepersistent

Copilot AI review requested due to automatic review settings April 15, 2026 13:02
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Apr 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.persistent parsing/validation to DatabaseSync::Prepare() and pass SQLITE_PREPARE_PERSISTENT via sqlite3_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() to sqlite3_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.

Comment thread src/node_sqlite.cc Outdated
@araujogui
araujogui force-pushed the sqlite-prepare-persistent branch from 504bd95 to 4f2ef78 Compare April 15, 2026 13:26
@araujogui

Copy link
Copy Markdown
Member Author

Interesting thing: SQLITE_PREPARE_PERSISTENT is enabled by default on better-sqlite3

@codecov

codecov Bot commented Apr 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.33%. Comparing base (1662b69) to head (d32e5e6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 90.90% 1 Missing and 1 partial ⚠️
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     
Files with missing lines Coverage Δ
src/node_sqlite.cc 81.68% <90.90%> (+0.05%) ⬆️

... and 37 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@louwers

louwers commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

At least makes sense to enable it by default for SQLTagStore statements I'd say.

@araujogui

Copy link
Copy Markdown
Member Author

@nodejs/sqlite Friendly bump

Comment thread test/parallel/test-sqlite-statement-sync.js Outdated
@araujogui
araujogui force-pushed the sqlite-prepare-persistent branch 2 times, most recently from 125d667 to 69fc714 Compare May 27, 2026 18:25
@meixg meixg added the request-ci Add this label to start a Jenkins CI on a PR. label May 29, 2026
@trivikr

trivikr commented Aug 12, 2026

Copy link
Copy Markdown
Member

@araujogui Can you please rebase from main?

@araujogui
araujogui force-pushed the sqlite-prepare-persistent branch from 69fc714 to d32e5e6 Compare August 13, 2026 17:44
@araujogui

Copy link
Copy Markdown
Member Author

@araujogui Can you please rebase from main?

Rebased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants