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
Local<Name> key;
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
if (owner != nullptr) {
// Reuses the statement's internalized column names instead of

@araujogui araujogui Aug 14, 2026

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.

maybe we can cache column names only if persistent is true to avoid caching short-lived statements.

@geeksilva97 geeksilva97 Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that's an interesting idea. However, IMO, such a change fits caching itself which would be separate change, while this PR is about leveraging cache implementation in paths they are not used yet.

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
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. wip Issues and PRs that are still a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants