-
Notifications
You must be signed in to change notification settings - Fork 30.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sqlite: allow returning ArrayBufferView
s from user-defined functions
#56790
sqlite: allow returning ArrayBufferView
s from user-defined functions
#56790
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #56790 +/- ##
=======================================
Coverage 89.21% 89.21%
=======================================
Files 663 663
Lines 192012 192012
Branches 36929 36931 +2
=======================================
+ Hits 171296 171300 +4
- Misses 13577 13580 +3
+ Partials 7139 7132 -7
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
It looks like this should be able to land but the commit queue is not picking it up - possibly because GitHub thinks there are still two pending checks? |
9fb613e
to
29ff781
Compare
Commit Queue failed- Loading data for nodejs/node/pull/56790 ✔ Done loading data for nodejs/node/pull/56790 ----------------------------------- PR info ------------------------------------ Title sqlite: allow returning `ArrayBufferView`s from user-defined functions (#56790) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch Renegade334:sqlite-function-arraybufferview -> nodejs:main Labels c++, semver-minor, author ready, needs-ci, sqlite Commits 1 - sqlite: allow returning `ArrayBufferView`s from user-defined functions Committers 1 - Renegade334 <contact.9a5d6388@renegade334.me.uk> PR-URL: https://github.com/nodejs/node/pull/56790 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/56790 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last approving review: ⚠ - sqlite: allow returning `ArrayBufferView`s from user-defined functions ℹ This PR was created on Mon, 27 Jan 2025 15:30:10 GMT ✔ Approvals: 3 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/56790#pullrequestreview-2580127244 ✔ - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/56790#pullrequestreview-2581083910 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/56790#pullrequestreview-2581360993 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2025-01-29T20:29:00Z: https://ci.nodejs.org/job/node-test-pull-request/64832/ ⚠ Commits were pushed after the last Full PR CI run: ⚠ - sqlite: allow returning `ArrayBufferView`s from user-defined functions - Querying data for job/node-test-pull-request/64832/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/13065470909 |
Oops. Can this be re-added to commit-queue? |
It needs to go through a full CI again since a commit was pushed. I'll kick one off. |
This comment was marked as outdated.
This comment was marked as outdated.
Landed in 2bd5694 |
PR-URL: #56790 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Notable changes: crypto: * (SEMVER-MINOR) support --use-system-ca on Windows (Joyee Cheung) #56833 * (SEMVER-MINOR) added support for reading certificates from macOS system store (Tim Jacomb) #56599 deps: * update timezone to 2025a (Node.js GitHub Bot) #56876 * (SEMVER-MINOR) update ada to v3.0.1 (Yagiz Nizipli) #56452 deps,tools: * (SEMVER-MINOR) add zstd 1.5.6 (Jan Krems) #52100 sqlite: * (SEMVER-MINOR) allow returning `ArrayBufferView`s from user-defined functions (René) #56790 src: * set signal inspector io thread name (RafaelGSS) #56416 * set thread name for main thread and v8 worker (RafaelGSS) #56416 * set worker thread name using worker.name (RafaelGSS) #56416 * use a default thread name for inspector (RafaelGSS) #56416 src, quic: * (SEMVER-MINOR) refine more of the quic implementation (James M Snell) #56328 test: * (SEMVER-MINOR) add WPT for URLPattern (Yagiz Nizipli) #56452 url: * (SEMVER-MINOR) add URLPattern implementation (Yagiz Nizipli) #56452 zlib: * (SEMVER-MINOR) add zstd support (Jan Krems) #52100 PR-URL: TODO
Notable changes: crypto: * (SEMVER-MINOR) support --use-system-ca on Windows (Joyee Cheung) #56833 * (SEMVER-MINOR) added support for reading certificates from macOS system store (Tim Jacomb) #56599 deps: * update timezone to 2025a (Node.js GitHub Bot) #56876 * (SEMVER-MINOR) update ada to v3.0.1 (Yagiz Nizipli) #56452 deps,tools: * (SEMVER-MINOR) add zstd 1.5.6 (Jan Krems) #52100 sqlite: * (SEMVER-MINOR) allow returning `ArrayBufferView`s from user-defined functions (René) #56790 src: * set signal inspector io thread name (RafaelGSS) #56416 * set thread name for main thread and v8 worker (RafaelGSS) #56416 * set worker thread name using worker.name (RafaelGSS) #56416 * use a default thread name for inspector (RafaelGSS) #56416 src, quic: * (SEMVER-MINOR) refine more of the quic implementation (James M Snell) #56328 test: * (SEMVER-MINOR) add WPT for URLPattern (Yagiz Nizipli) #56452 url: * (SEMVER-MINOR) add URLPattern implementation (Yagiz Nizipli) #56452 zlib: * (SEMVER-MINOR) add zstd support (Jan Krems) #52100 PR-URL: #57005
#56385 widened the valid "blob-equivalent" type from
Uint8Array
toArrayBufferView
for bound parameters. This isn't currently reflected in the valid return types for user-defined functions, and it seems sensible to keep things consistent.