Skip to content

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

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
geeksilva97:post/column-name-cache
Aug 18, 2026
Merged

sqlite: reuse cached column names in all() and get()#65276
nodejs-github-bot merged 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 53 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

@mcollina mcollina 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

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@geeksilva97 geeksilva97 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 16, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 17, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 17, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/65276
✔  Done loading data for nodejs/node/pull/65276
----------------------------------- PR info ------------------------------------
Title      sqlite: reuse cached column names in all() and get() (#65276)
Author     Edy Silva <edigleyssonsilva@gmail.com> (@geeksilva97)
Branch     geeksilva97:post/column-name-cache -> nodejs:main
Labels     c++, author ready, needs-ci, commit-queue, sqlite
Commits    1
 - sqlite: reuse cached column names in statement all() and get()
Committers 1
 - geeksilva97 <edigleyssonsilva@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65276
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65276
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 14 Aug 2026 03:45:09 GMT
   ✔  Approvals: 3
   ✔  - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/65276#pullrequestreview-4944096915
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/65276#pullrequestreview-4944479607
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/65276#pullrequestreview-4947917214
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2026-08-17T05:01:25Z: https://ci.nodejs.org/job/node-test-pull-request/75907/
- Querying data for job/node-test-pull-request/75907/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/32002702127

@geeksilva97 geeksilva97 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 17, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 17, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/65276
✔  Done loading data for nodejs/node/pull/65276
----------------------------------- PR info ------------------------------------
Title      sqlite: reuse cached column names in all() and get() (#65276)
Author     Edy Silva <edigleyssonsilva@gmail.com> (@geeksilva97)
Branch     geeksilva97:post/column-name-cache -> nodejs:main
Labels     c++, author ready, needs-ci, commit-queue, sqlite
Commits    1
 - sqlite: reuse cached column names in statement all() and get()
Committers 1
 - geeksilva97 <edigleyssonsilva@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65276
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65276
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 14 Aug 2026 03:45:09 GMT
   ✔  Approvals: 3
   ✔  - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/65276#pullrequestreview-4944096915
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/65276#pullrequestreview-4944479607
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/65276#pullrequestreview-4947917214
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2026-08-17T06:43:11Z: https://ci.nodejs.org/job/node-test-pull-request/75907/
- Querying data for job/node-test-pull-request/75907/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/32022868997

@trivikr

This comment was marked as outdated.

@geeksilva97 geeksilva97 added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 18, 2026
@nodejs-github-bot
nodejs-github-bot merged commit cf30b2e into nodejs:main Aug 18, 2026
116 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in cf30b2e

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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.

6 participants