Skip to content

sqlite: validate StatementSync.run() integers - #65178

Open
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:sqlite-statement-sync-lastinsertrowid-precision
Open

sqlite: validate StatementSync.run() integers#65178
trivikr wants to merge 2 commits into
nodejs:mainfrom
trivikr:sqlite-statement-sync-lastinsertrowid-precision

Conversation

@trivikr

@trivikr trivikr commented Aug 10, 2026

Copy link
Copy Markdown
Member

Fixes: #65177

Use the standard SQLite integer conversion for changes and lastInsertRowid. Throw ERR_OUT_OF_RANGE when a value cannot be represented safely as a Number, or return it as a BigInt when BigInt reads are enabled.


Assisted-by: codex:gpt-5.6-sol

@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 Aug 10, 2026
Use the standard SQLite integer conversion for changes and
lastInsertRowid. Throw ERR_OUT_OF_RANGE when a value cannot be
represented safely as a Number, or return it as a BigInt when
BigInt reads are enabled.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@trivikr
trivikr force-pushed the sqlite-statement-sync-lastinsertrowid-precision branch from fb954ab to 01f2928 Compare August 10, 2026 04:25
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2026
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.32%. Comparing base (6fec5e8) to head (e07659a).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #65178   +/-   ##
=======================================
  Coverage   90.32%   90.32%           
=======================================
  Files         760      760           
  Lines      248523   248534   +11     
  Branches    46896    46897    +1     
=======================================
+ Hits       224469   224492   +23     
+ Misses      15465    15464    -1     
+ Partials     8589     8578   -11     
Files with missing lines Coverage Δ
src/node_sqlite.cc 81.25% <92.30%> (+0.01%) ⬆️

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

Comment thread src/node_sqlite.cc Outdated

@araujogui araujogui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@trivikr
trivikr requested a review from Qard August 12, 2026 01:06
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.

sqlite: StatementSync.run() loses precision for lastInsertRowid instead of throwing ERR_OUT_OF_RANGE

3 participants