sqlite: add setReturnArrays method to StatementSync by gurgunday · Pull Request #57542 · nodejs/node · GitHub
Skip to content

sqlite: add setReturnArrays method to StatementSync#57542

Merged
nodejs-github-bot merged 15 commits intonodejs:mainfrom
gurgunday:sqlite-array
Apr 4, 2025
Merged

sqlite: add setReturnArrays method to StatementSync#57542
nodejs-github-bot merged 15 commits intonodejs:mainfrom
gurgunday:sqlite-array

Conversation

@gurgunday
Copy link
Copy Markdown
Member

@gurgunday gurgunday commented Mar 19, 2025

Add a new StatementSync method setReturnArrays() that allows query results to be returned as arrays instead of objects. This is more efficient when column names are meaningless, auto-generated, or when working with hundreds of columns.

Fixes #57534

It's my first significant C++ PR, hopefully first of many :)

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Mar 19, 2025

@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 Mar 19, 2025
@gurgunday gurgunday changed the title sqlite: add setReturnArrays method to StatementSync for toggling arra… sqlite: add setReturnArrays method to StatementSync for toggling array row support Mar 19, 2025
@gurgunday gurgunday changed the title sqlite: add setReturnArrays method to StatementSync for toggling array row support sqlite: add setReturnArrays method to StatementSync Mar 19, 2025
geeksilva97
geeksilva97 reviewed Mar 19, 2025
View reviewed changes
Comment thread src/node_sqlite.cc Outdated
@anonrig anonrig requested review from anonrig, cjihrig and jasnell March 19, 2025 01:30
@anonrig
Copy link
Copy Markdown
Member

anonrig commented Mar 19, 2025

Cc @nodejs/cpp-reviewers

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 19, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.85859% with 14 lines in your changes missing coverage. Please review.

Project coverage is 90.25%. Comparing base (32e5e81) to head (5420ed5).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 85.85% 0 Missing and 14 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #57542      +/-   ##
==========================================
+ Coverage   90.24%   90.25%   +0.01%     
==========================================
  Files         630      630              
  Lines      184990   185042      +52     
  Branches    36214    36232      +18     
==========================================
+ Hits       166941   167008      +67     
+ Misses      11002    10996       -6     
+ Partials     7047     7038       -9
Files with missing lines Coverage Δ
src/node_sqlite.h 63.63% <ø> (ø)
src/node_sqlite.cc 80.52% <85.85%> (+<0.01%) ⬆️

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

cjihrig
cjihrig reviewed Mar 19, 2025
View reviewed changes
Comment thread test/parallel/test-sqlite-statement-sync.js
Comment thread test/parallel/test-sqlite-statement-sync.js Outdated
Comment thread test/parallel/test-sqlite-statement-sync.js Outdated
Comment thread src/node_sqlite.cc Outdated
Comment thread src/node_sqlite.cc Outdated
Comment thread src/node_sqlite.cc Outdated
@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Mar 19, 2025

Oh, and this needs documentation.

anonrig
anonrig reviewed Mar 19, 2025
View reviewed changes
Comment thread src/node_sqlite.cc Outdated
Comment thread src/node_sqlite.cc Outdated
Comment thread src/node_sqlite.cc Outdated
@gurgunday
Copy link
Copy Markdown
Member Author

gurgunday commented Mar 19, 2025 •
edited
Loading

I'll add docs once the code is approved

@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Mar 20, 2025

I've opened #57569 to refactor the iterator implementation. I think we should land that PR before this one so that we can avoid exposing iterator internal state to JS.

@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Apr 3, 2025

This needs a rebase, but we can finally move this forward.

@gurgunday
Copy link
Copy Markdown
Member Author

gurgunday commented Apr 3, 2025

Yep, will rebase asap

@cjihrig cjihrig added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Apr 4, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 4, 2025
@nodejs-github-bot nodejs-github-bot merged commit 054371d into nodejs:main Apr 4, 2025
67 checks passed
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Apr 4, 2025

Landed in 054371d

@gurgunday gurgunday deleted the sqlite-array branch April 4, 2025 23:20
JonasBa pushed a commit to JonasBa/node that referenced this pull request Apr 11, 2025
@gurgunday @JonasBa
sqlite: add setReturnArrays method to StatementSync
255ce35 
PR-URL: nodejs#57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@gurgunday @RafaelGSS
sqlite: add setReturnArrays method to StatementSync
23b2bc9 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@gurgunday @RafaelGSS
sqlite: add setReturnArrays method to StatementSync
2b2a0bf 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
0b50262 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
4a8b246 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@gurgunday @RafaelGSS
sqlite: add setReturnArrays method to StatementSync
ac6f7d1 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
688a954 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
9a67be8 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
e8ec01d 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
f996ab0 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
ad464f3 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@github-actions github-actions Bot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@gurgunday @aduh95
sqlite: add setReturnArrays method to StatementSync
5e0503a 
PR-URL: #57542
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@ghost ghost mentioned this pull request Jun 8, 2025
Closed
@tniessen
Copy link
Copy Markdown
Member

tniessen commented Jun 18, 2025

@aduh95 aduh95 mentioned this pull request Jul 17, 2025
Merged
aduh95 pushed a commit that referenced this pull request Jul 17, 2025
@geeksilva97
doc: add missing section for setReturnArrays in sqlite.md
ee1ec50 
PR-URL: #59074
Refs: #57542
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit to geeksilva97/node that referenced this pull request Jul 17, 2025
@geeksilva97 @aduh95
doc: add missing section for setReturnArrays in sqlite.md
8eaa19a 
PR-URL: nodejs#59074
Refs: nodejs#57542
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 17, 2025
@geeksilva97 @aduh95
doc: add missing section for setReturnArrays in sqlite.md
34c4900 
PR-URL: #59074
Refs: #57542
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 21, 2025
@geeksilva97
doc: add missing section for setReturnArrays in sqlite.md
997171d 
PR-URL: #59074
Backport-PR-URL: #59099
Refs: #57542
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 21, 2025
@geeksilva97 @aduh95
doc: add missing section for setReturnArrays in sqlite.md
f7924a1 
PR-URL: #59074
Backport-PR-URL: #59099
Refs: #57542
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 25, 2025
@geeksilva97 @aduh95
doc: add missing section for setReturnArrays in sqlite.md
343dbc8 
PR-URL: #59074
Backport-PR-URL: #59099
Refs: #57542
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 26, 2025
@geeksilva97 @aduh95
doc: add missing section for setReturnArrays in sqlite.md
e22aeaa 
PR-URL: #59074
Backport-PR-URL: #59099
Refs: #57542
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@EtanHey EtanHey mentioned this pull request Mar 15, 2026
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anonrig anonrig anonrig left review comments

@geeksilva97 geeksilva97 geeksilva97 left review comments

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell Awaiting requested review from jasnell

Assignees

No one assigned

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++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

node:sqlite to support retrieving rows as arrays instead of objects

6 participants

@gurgunday @nodejs-github-bot @anonrig @cjihrig @tniessen @geeksilva97