Skip to content

sqlite: use shared-shape objects for result rows - #66384

Closed
araujogui wants to merge 1 commit into
nodejs:mainfrom
araujogui:sqlite-row-template
Closed

araujogui wants to merge 1 commit into
nodejs:mainfrom
araujogui:sqlite-row-template

Conversation

@araujogui

Copy link
Copy Markdown
Member

all(), get() and iterate() built each row with the Object::New() overload that takes names and values. That overload always returns a dictionary-mode object, so no two rows shared a map and every property read was a hash lookup.

Rows are now built from a DictionaryTemplate cached on the statement and invalidated on re-prepare. Rows keep their null prototype. Statements whose column names a template can't express (array indices, duplicates, non-ASCII names, which the template interns as Latin-1) or with more than 64 columns keep the previous path.

Adds benchmark/sqlite/sqlite-prepare-select-read.js, which reads every column of each row, since the existing benchmarks only measure building rows.

Fixes: #65799

Rows returned by get(), all() and iterate() were created with
Object::New() and a null prototype. That produces a dictionary-mode
object with its own property dictionary for every row.

Cache a DictionaryTemplate per statement, rebuilt when the statement
is re-prepared, so rows get fast properties and share a map. Column
names that DictionaryTemplate cannot represent keep using
Object::New(): array indices, duplicates, and non-ASCII names, which
it would intern as Latin-1.

Assisted-by: Claude Code
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Copilot AI balanced review requested due to automatic review settings September 28, 2026 23:38
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/sqlite

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@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 28, 2026
@araujogui araujogui closed this Sep 28, 2026
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.24590% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.36%. Comparing base (ae9c25a) to head (db6b6b7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 85.24% 3 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66384      +/-   ##
==========================================
+ Coverage   90.35%   90.36%   +0.01%     
==========================================
  Files         792      792              
  Lines      275434   275477      +43     
  Branches    52781    52791      +10     
==========================================
+ Hits       248868   248942      +74     
+ Misses      16978    16939      -39     
- Partials     9588     9596       +8     
Files with missing lines Coverage Δ
src/node_sqlite.h 87.27% <ø> (ø)
src/node_sqlite.cc 82.09% <85.24%> (+0.13%) ⬆️

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: remove the null prototype from result rows

3 participants