Skip to content

sqlite: add stmt persistent flag#62757

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

sqlite: add stmt persistent flag#62757
araujogui wants to merge 3 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
Copy link
Copy Markdown

Copilot AI left a comment

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
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.32%. Comparing base (8257091) to head (69fc714).
⚠️ Report is 3 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.32%   90.32%           
=======================================
  Files         730      730           
  Lines      234671   234690   +19     
  Branches    43946    43957   +11     
=======================================
+ Hits       211965   211993   +28     
+ Misses      14423    14420    -3     
+ Partials     8283     8277    -6     
Files with missing lines Coverage Δ
src/node_sqlite.cc 80.68% <90.90%> (+0.06%) ⬆️

... and 35 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
Copy link
Copy Markdown
Contributor

louwers commented Apr 15, 2026

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 from 5de847e to 125d667 Compare May 27, 2026 18:18
@araujogui araujogui force-pushed the sqlite-prepare-persistent branch from 125d667 to 69fc714 Compare May 27, 2026 18:25
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. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants