Skip to content

sqlite: bind undefined to NULL - #65709

Open
TrevorBurnham wants to merge 1 commit into
nodejs:mainfrom
TrevorBurnham:sqlite-bind-undefined-to-null
Open

sqlite: bind undefined to NULL#65709
TrevorBurnham wants to merge 1 commit into
nodejs:mainfrom
TrevorBurnham:sqlite-bind-undefined-to-null

Conversation

@TrevorBurnham

@TrevorBurnham TrevorBurnham commented Sep 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #61824

This PR re-implements #62008 by @mike-git374. The one-line behavior change is theirs; this PR adds the documentation and the remaining test coverage.

Omitting a named parameter binds NULL, but passing undefined for that same parameter threw ERR_INVALID_ARG_TYPE:

s.run({ k: 1 });               // v binds to NULL
s.run({ k: 2, v: undefined }); // throws ERR_INVALID_ARG_TYPE

Binding undefined to NULL makes those agree. It also matches JSValueToSQLiteResult, which already maps a user-defined function's undefined return value to NULL, and SQLite's own WASM oo1 API.
@nodejs-github-bot

nodejs-github-bot commented Sep 1, 2026

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 Sep 1, 2026
@TrevorBurnham @mike-git374
sqlite: bind undefined to NULL
47bbca8 
Omitting a named parameter binds NULL, but passing `undefined` for that
same parameter threw ERR_INVALID_ARG_TYPE. Bind `undefined` to NULL so
the two forms agree. This matches the conversion already applied to a
user-defined function's `undefined` return value, as well as SQLite's
own WASM oo1 API.

Fixes: nodejs#61824
Refs: nodejs#61472
Refs: nodejs#62008
Co-authored-by: mike-git374 <217764531+mike-git374@users.noreply.github.com>
Assisted-by: claude:opus-5
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
@TrevorBurnham
TrevorBurnham force-pushed the sqlite-bind-undefined-to-null branch from 5cb500c to 47bbca8 Compare September 1, 2026 15:45
@TrevorBurnham
TrevorBurnham marked this pull request as ready for review September 1, 2026 15:51
@TrevorBurnham TrevorBurnham mentioned this pull request Sep 1, 2026
Closed
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.04%. Comparing base (f9ab994) to head (47bbca8).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #65709      +/-   ##
==========================================
- Coverage   90.07%   90.04%   -0.03%     
==========================================
  Files         754      754              
  Lines      256395   256395              
  Branches    48494    48499       +5     
==========================================
- Hits       230947   230875      -72     
- Misses      16563    16639      +76     
+ Partials     8885     8881       -4
Files with missing lines Coverage Δ
src/node_sqlite.cc 82.05% <100.00%> (+0.06%) ⬆️

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

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.

2 participants

@TrevorBurnham @nodejs-github-bot