Skip to content

sqlite: avoid sqlite3_clear_bindings() before every run - #66983

Open
dualfroz wants to merge 1 commit into
nodejs:mainfrom
dualfroz:sqlite-clear-bindings
Open

dualfroz wants to merge 1 commit into
nodejs:mainfrom
dualfroz:sqlite-clear-bindings

Conversation

@dualfroz

Copy link
Copy Markdown

BindParams called sqlite3_clear_bindings() before every run, which since SQLite 3.54 invalidates the plan, so JOIN queries got re-prepared each time. it now tracks which parameters were bound and binds NULL only to the rest, so omitted params are still NULL. ResetAndBindStatement always binds every parameter, so it doesnt clear at all. added a test for omitted named params, including { k, $k } hitting the same index.

note this only removes the re-prepare for integer (BigInt) bindings. plain JS numbers go through sqlite3_bind_double, which in 3.54 still invalidates the plan on every bind, so the case from the issue gets only a bit faster (3.4-4.9 to 3.1-3.2 us per get). BigInt goes from 3.4-4.3 to ~0.7 us.

Refs: #66800

Since SQLite 3.54, sqlite3_clear_bindings() invalidates the query plan
of the statement, so a statement was re-prepared on every run. Bind NULL
only to the parameters that were not bound in this run instead, and skip
clearing in the tag store, which always binds every parameter.

Refs: nodejs#66800
Signed-off-by: dualfroz <me@dualfroz.com>
@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 Oct 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

Caution

AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context.

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 25, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 25, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@codecov

codecov Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.59%. Comparing base (91c4436) to head (79ed629).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66983      +/-   ##
==========================================
- Coverage   92.93%   90.59%   -2.35%     
==========================================
  Files         428      797     +369     
  Lines      195236   278304   +83068     
  Branches    30176    53513   +23337     
==========================================
+ Hits       181442   252118   +70676     
- Misses      13467    16570    +3103     
- Partials      327     9616    +9289     
Files with missing lines Coverage Δ
src/node_sqlite.cc 82.12% <90.90%> (ø)

... and 500 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.

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

Labels

agentscan:automated-account 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.

4 participants