Skip to content

sqlite: reuse cached column names in all() and get() - #65276

Open
geeksilva97 wants to merge 1 commit into
nodejs:mainfrom
geeksilva97:post/column-name-cache
Open

sqlite: reuse cached column names in all() and get()#65276
geeksilva97 wants to merge 1 commit into
nodejs:mainfrom
geeksilva97:post/column-name-cache

Conversation

@geeksilva97

@geeksilva97 geeksilva97 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Makes StatementSync.prototype.all and StatementSync.prototype.get to leverage existing cache

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@geeksilva97 geeksilva97 added the wip Issues and PRs that are still a work in progress. label Aug 14, 2026
@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 14, 2026
@geeksilva97
geeksilva97 force-pushed the post/column-name-cache branch from c7fbb27 to 2822d7d Compare August 14, 2026 03:48
Comment thread src/node_sqlite.cc Outdated
Signed-off-by: geeksilva97 <edigleyssonsilva@gmail.com>
@geeksilva97
geeksilva97 force-pushed the post/column-name-cache branch from 2822d7d to c83e627 Compare August 14, 2026 13:37
@geeksilva97
geeksilva97 marked this pull request as ready for review August 14, 2026 13:49

@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

@geeksilva97 geeksilva97 removed the wip Issues and PRs that are still a work in progress. label Aug 14, 2026
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.32%. Comparing base (91a99c5) to head (c83e627).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 66.66% 2 Missing and 5 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #65276   +/-   ##
=======================================
  Coverage   90.32%   90.32%           
=======================================
  Files         751      751           
  Lines      250000   249977   -23     
  Branches    47231    47226    -5     
=======================================
- Hits       225816   225803   -13     
- Misses      15566    15571    +5     
+ Partials     8618     8603   -15     
Files with missing lines Coverage Δ
src/node_sqlite.h 83.33% <ø> (ø)
src/node_sqlite.cc 81.26% <66.66%> (-0.37%) ⬇️

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

@geeksilva97
geeksilva97 requested a review from mcollina August 14, 2026 18:57
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.

3 participants