sqlite: add setReturnArrays method to StatementSync #57542
Conversation
|
Review requested:
|
nodejs-github-bot
added
c++
gurgunday
changed the title
gurgunday
changed the title
|
Cc @nodejs/cpp-reviewers |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57542 +/- ##
==========================================
- Coverage 90.23% 90.23% -0.01%
==========================================
Files 629 629
Lines 184939 185009 +70
Branches 36232 36244 +12
==========================================
+ Hits 166885 166940 +55
- Misses 11011 11016 +5
- Partials 7043 7053 +10
🚀 New features to boost your workflow:
|
| t.assert.strictEqual(setup, undefined); | ||
|
|
||
| const query = db.prepare('SELECT key, val FROM data WHERE key = 1'); | ||
| t.assert.deepStrictEqual(query.get(), { __proto__: null, key: 1, val: 'one' }); |
There was a problem hiding this comment.
It might be worth having dedicated test cases for get(), all(), and iterate() so that no one comes along in the future and refactors what might seem like an unnecessarily complex test case.
There was a problem hiding this comment.
Should we change the tests or just add a different suite for them?
There was a problem hiding this comment.
The tests themselves look fine. I would just split them up.
|
Oh, and this needs documentation. |
|
I'll add docs once the code is approved |
|
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. |
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 :)