sqlite: add support for SQLite Session Extension

[louwers]

We were talking about adding support for the Session Extension of SQLite in #53752. This is not a run-time extension but rather a feature built in to the SQLite amalgamation. It can be enabled with a compile flag and is enabled by default on some platforms.

I have never contributed to Node.js before so I will probably need some help to bring this to a conclusion.

---

TODO:

• Make sure sessions are deleted before the database they are attached to is closed
• Consensus on API
• Documentation
• Allow custom conflict handler
• Throw with specific (documented) exception in case of conflict when applying changeset since SQLite doesn't consider this to be an error I return false when applying the changeset is aborted due to a conflict
• Allow generating a patchset as well
• Allow specifying table name when creating session (to only track that table)
• Implement Session.close()

---

Example usage:

const database1 = new DatabaseSync(':memory:');
const database2 = new DatabaseSync(':memory:');

database1.exec('CREATE TABLE data(key INTEGER PRIMARY KEY, value TEXT)');
database2.exec('CREATE TABLE data(key INTEGER PRIMARY KEY, value TEXT)');

const session = database1.createSession();

const insert = database1.prepare('INSERT INTO data (key, value) VALUES (?, ?)');
insert.run(1, 'hello');
insert.run(2, 'world');

const changeset = session.changeset();
database2.applyChangeset(changeset);
// Now database2 contains the same data as database1 

[nodejs-github-bot]

Review requested:

• @nodejs/gyp
• @nodejs/security-wg

[karimfromjordan]

Wouldn't this be solved if users could use their own SQLite build? That's already being considered according to the original issue.

[louwers]

@karimfromjordan Nope, because you need access to the C API to access this feature.

[anonrig]

Can you add the appropriate documentation as well?

[codecov]

Codecov Report

Attention: Patch coverage is 86.06965% with 28 lines in your changes missing coverage. Please review.

Project coverage is 88.41%. Comparing base (7b01758) to head (092a28c).
Report is 101 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_sqlite.cc	86.93%	6 Missing and 20 partials ⚠️
src/node_sqlite.h	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54181      +/-   ##
==========================================
+ Coverage   88.40%   88.41%   +0.01%     
==========================================
  Files         654      654              
  Lines      187594   188034     +440     
  Branches    36089    36169      +80     
==========================================
+ Hits       165834   166259     +425     
+ Misses      15001    14999       -2     
- Partials     6759     6776      +17     

Files with missing lines	Coverage Δ
src/node_sqlite.h	70.00% <0.00%> (-7.78%)	⬇️
src/node_sqlite.cc	83.80% <86.93%> (+0.47%)	⬆️

... and 62 files with indirect coverage changes

[louwers]

I need to make sure sessions are deleted before the database they are attached to is closed.

I think I need the DatabaseSync hold onto the Sessions it creates. Both are BaseObjects so I need to look into how to do that.

When a Session goes out of scope the corresponding DatabaseSync needs to no longer hold onto it. Conversely I also need to make sure that all operations on Session objects are no-ops after the corresponding database is closed.

[louwers]

@anonrig I have added documentation, most of the functionality and fixed the lint issues. Could you trigger another CI run?

I want to add support for generating a patchset and then this PR is ready for review.

I have some ideas for future improvements, e.g. more fine-grained conflict handling or exposing the utilities SQLite provides for inspecting and merging changesets, but they can be added in a backward-compatible manner.

[TheOneTheOnlyJJ]

This is excellent work that many projects will surely be based on. I am curious about the eventual wrapping of the entire Sessions C API (mixed & mashed into higher-level Node functions). Does this PR leave space for that? There are a few more Sessions API functions that would be of great use in Node.

[louwers]

@TheOneTheOnlyJJ Thanks! I agree that exposing more functionality of the Session Extension in Node.js would be great. Here are some ideas:

• Wrappers for sqlite3changeset_invert() (creates a changeset that undoes the changes in another changeset) and sqlite3changeset_concat() (combines changesets).
• Allow closing a session. Right now a session is closed when it is garbage collected or when the corresponding database is closed. It is helpful to be able to manually close session in some cases.
• Allowing the contents of a changeset to be read. You probably want to reuturn a Generator for this so you don't need to read the entire changeset it in memory at once.
• Change the onConflict handler to take a function as well. You would be able to inspect the conflicting change and decide on the action (omit, replace, abort) for each conflicting change.
• A wrapper for sqlite3session_diff().
• Utilize streaming versions of the Session API (you would need another API for Node.js as well). These are helpful for reading or applying changesets that do not fit in memory.

Here is the full API for future reference: https://www.sqlite.org/session/funclist.html

Right now I first want to gather some feedback, but I'd be interested continue working on this, either in a follow-up PR or in this one. Since the SQLite integration is still under active development I think working incrementally is the preferred approach.

[TheOneTheOnlyJJ]

Good point, here's my view on them:

• Wrappers for sqlite3changeset_invert() (creates a changeset that undoes the changes in another changeset) and sqlite3changeset_concat() (combines changesets).

Yes, I was about to suggest these. There's also sqlite3session_isempty() and sqlite3session_memory_used() which are simpler (compared to the rest of the API) and could be wrapped more easily.

• Allow closing a session. Right now a session is closed when it is garbage collected or when the corresponding database is closed. It is helpful to be able to manually close session in some cases.

This should be a high priority right now, as long-running Node processes (like server back-ends) that would use Sessions will have their memory slowly but surely filled up with no way of freeing it without closing the database connection. Applications could run for months or even longer without closing a database connection, so not being able to close Sessions and free their memory would deter developers from using them. The database should, of course, still keep a list of all active sessions and delete them when it is closed, just in case not all Sessions were manually closed. From my point of view, this function should be wrapped as soon as possible and be included in this PR. Not having it surely decreases the chances of getting this merged, and it would be a shame to lose out on this.

• Allowing the contents of a changeset to be read. You probably want to reuturn a Generator for this so you don't need to read the entire changeset it in memory at once.
• Change the onConflict handler to take a function as well. You would be able to inspect the conflicting change and decide on the action (omit, replace, abort) for each conflicting change.
• A wrapper for sqlite3session_diff().

Very important functionalities, but again, they're not vital for achieving functional basic interaction with the extension. These 3 points could be bundled in a follow-up PR that handles the finer-grained data management side of the API. The changegroup could be exposed as an object with its API functions bound to it as methods.

• Utilize streaming versions of the Session API (you would need another API for Node.js as well). These are helpful for reading or applying changesets that do not fit in memory.

I've noticed this in the documentation and it's definitely a part of the API that would be crucial in large apps with large databases. But again, for the current goal of incrementally proposing a stable API that exposes the API of the extension, this is not required right now. It would also require wrapping sqlite3session_config(), which allows configuring SQLITE_SESSION_CONFIG_STRMSIZE. This could be handled after the 3 points above get sorted out.

Also, before I wrap up...

I noticed that you proposed the createSession() method as a wrapper around both sqlite3session_create() and sqlite3session_attach(), combined. Looking at the API, I'm trying to figure out if there's any potential functionality or use case where exposing these functions separately in Node would make sense. The only potential use case I see is creating Sessions in one part of the application and then attaching them somewhere else. Maybe this would be useful if using a global Session that gets created at app start and attached later in the app life cycle? But, given that a Session cannot be unattached, this is likely not going to be useful in any way, so the separation of the functions is not needed. What's you input on this, do you see any potential use cases for the splitting of these 2 functions?

[louwers]

This should be a high priority right now

Agreed. I will still add this one as part of this PR.

What's you input on this, do you see any potential use cases for the splitting of these 2 functions?

I don't see any benefits. Furthermore, I like the philosophy of better-sqlite3:

better-sqlite3's public API must be as simple as possible. Rather than calling 3 functions in a specific order, it's simpler for users to call a single function. Rather than providing many similar functions for doing similar things (e.g., "convenience functions"), there should just be one function that is already convenient by design. Sane defaults should be applied when possible. A function's minimal call signature should be as small as possible, with progressively complex customization available when needed. Function names should only be as long as necessary to convey their purpose. For any new feature, it should be easy to showcase code examples that is are so simple that they are self-explanatory.

In this vein I also oped to default to aborting on conflict, instead of requiring an explicit conflict handler to be passed like in the API of SQLite.

[TheOneTheOnlyJJ]

Agreed. I will still add this one as part of this PR.

Yes, this would be ideal. For further updates other than this, I believe it's best to wait for the final form of the node:sqlite API itself.

I don't see any benefits. Furthermore, I like the philosophy of better-sqlite3:

I was checking if any additional functionality could be squeezed out by separating the functions. Given that there is none, your decision is a sensible one and I support it.

In this vein I also oped to default to aborting on conflict, instead of requiring an explicit conflict handler to be passed like in the API of SQLite.

This is also the best sane default option, as every developer should customize this behavior according to their use case.

I'm not experienced in the internals of Node, so my contribution here can only amount to discussing the API design and smaller implementation details.

I guess the next step now (after implementing the Session close wrapper) is to wait for a maintainer to review this?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63585/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63596/

[louwers]

Happy to see that CI is green! I made one more small fix to address a PR comment from @jasnell to include strings in the environment.

Let me know if there's anything else. 🙏

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/63603/

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/54181
✔  Done loading data for nodejs/node/pull/54181
----------------------------------- PR info ------------------------------------
Title      sqlite: add support for SQLite Session Extension (#54181)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     louwers:session-extension -> nodejs:main
Labels     c++, lib / src, author ready, sqlite
Commits    31
 - sqlite: add support for SQLite Session Extension
 - doc: address PR comment @RedYetiDev
 - sqlite: keep MemoryInfo related stuff together
 - sqlite: use snake_case for local variables
 - sqlite: remove unneccesary use of node namespace
 - sqlite: improve working filter parameter
 - sqlite: add comment about using static functions
 - doc: improve wording default value onConflict
 - doc: capitalize Default
 - doc: improve wording options documentation
 - doc: improve options documentation
 - doc: improve wording table parameter
 - test: improve test name
 - sqlite: move SQLITE_ENABLE_SESSION to node.gyp
 - test: split out sqlite session test and fix null __proto__
 - sqlite:  use FIXED_ONE_BYTE_STRING instead of NewFromUtf8
 - sqlite: don't use ToLocalChecked
 - sqlite: avoid double check
 - doc: add documentation for constants
 - doc: add explanation database name
 - test: remove unused imports test-sqlite.js
 - test: add some extra asserts for changeset and patchset
 - test: create suite for conflict resolution
 - test: fix deepStrictEqual helper
 - test: fix lint issues
 - doc: fix lint issue
 - test: format import
 - sqlite: run format
 - sqlite: add filter and onConflict to env_properties.h
 - sqlite: format
 - sqlite: simplify string conversion
Committers 2
 - Bart Louwers <bart@emeel.net>
 - GitHub <noreply@github.com>
PR-URL: https://github.com/nodejs/node/pull/54181
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/54181
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 02 Aug 2024 17:57:31 GMT
   ✔  Approvals: 3
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/54181#pullrequestreview-2217182204
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/54181#pullrequestreview-2417021902
   ✔  - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/54181#pullrequestreview-2441289456
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-11-17T14:29:53Z: https://ci.nodejs.org/job/node-test-pull-request/63603/
- Querying data for job/node-test-pull-request/63603/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 54181
From https://github.com/nodejs/node
 * branch                  refs/pull/54181/merge -> FETCH_HEAD
✔  Fetched commits as d159c9732022..092a28c38383
--------------------------------------------------------------------------------
Auto-merging doc/api/sqlite.md
Auto-merging src/env_properties.h
Auto-merging src/node_sqlite.cc
[main ebd8540f5e] sqlite: add support for SQLite Session Extension
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 19:17:33 2024 +0100
 7 files changed, 766 insertions(+), 18 deletions(-)
Auto-merging doc/api/sqlite.md
[main c5f293f53d] doc: address PR comment @RedYetiDev
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 19:21:26 2024 +0100
 1 file changed, 8 insertions(+), 10 deletions(-)
[main 2fa11850c2] sqlite: keep MemoryInfo related stuff together
 Author: Bart Louwers <bart.louwers@gmail.com>
 Date: Sat Nov 2 19:24:26 2024 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging src/node_sqlite.cc
[main 2947d277f7] sqlite: use snake_case for local variables
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 19:27:58 2024 +0100
 1 file changed, 3 insertions(+), 3 deletions(-)
Auto-merging src/node_sqlite.cc
[main f2b2725274] sqlite: remove unneccesary use of node namespace
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 19:38:17 2024 +0100
 1 file changed, 36 insertions(+), 38 deletions(-)
Auto-merging doc/api/sqlite.md
[main 899ca75f64] sqlite: improve working filter parameter
 Author: Bart Louwers <bart.louwers@gmail.com>
 Date: Sat Nov 2 19:53:16 2024 +0100
 1 file changed, 1 insertion(+), 3 deletions(-)
Auto-merging src/node_sqlite.cc
[main 44551c1a35] sqlite: add comment about using static functions
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 19:41:34 2024 +0100
 1 file changed, 2 insertions(+)
Auto-merging doc/api/sqlite.md
[main 48db7181a8] doc: improve wording default value onConflict
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 19:56:08 2024 +0100
 1 file changed, 2 insertions(+), 2 deletions(-)
Auto-merging doc/api/sqlite.md
[main 21b060895c] doc: capitalize Default
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 19:57:27 2024 +0100
 1 file changed, 2 insertions(+), 2 deletions(-)
Auto-merging doc/api/sqlite.md
[main 290de00f82] doc: improve wording options documentation
 Author: Bart Louwers <bart.louwers@gmail.com>
 Date: Sat Nov 2 19:57:53 2024 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging doc/api/sqlite.md
[main 83958e87e8] doc: improve options documentation
 Author: Bart Louwers <bart.louwers@gmail.com>
 Date: Sat Nov 2 19:58:48 2024 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
Auto-merging doc/api/sqlite.md
[main 913d312858] doc: improve wording table parameter
 Author: Bart Louwers <bart.louwers@gmail.com>
 Date: Sat Nov 2 19:59:45 2024 +0100
 1 file changed, 1 insertion(+), 2 deletions(-)
[main 2d2aa297b6] test: improve test name
 Author: Bart Louwers <bart.louwers@gmail.com>
 Date: Sat Nov 2 20:00:12 2024 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
[main ceb4a8951a] sqlite: move SQLITE_ENABLE_SESSION to node.gyp
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 22:01:38 2024 +0100
 2 files changed, 1 insertion(+), 2 deletions(-)
[main 6a7a3bca38] test: split out sqlite session test and fix null __proto__
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 22:28:23 2024 +0100
 2 files changed, 368 insertions(+), 350 deletions(-)
 create mode 100644 test/parallel/test-sqlite-session.js
Auto-merging src/node_sqlite.cc
[main c785a23532] sqlite:  use FIXED_ONE_BYTE_STRING instead of NewFromUtf8
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 22:50:35 2024 +0100
 1 file changed, 1 insertion(+), 3 deletions(-)
Auto-merging src/node_sqlite.cc
[main 9af718ebbb] sqlite: don't use ToLocalChecked
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 22:52:16 2024 +0100
 1 file changed, 4 insertions(+), 2 deletions(-)
Auto-merging src/node_sqlite.cc
[main e270f8d95d] sqlite: avoid double check
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 23:18:02 2024 +0100
 1 file changed, 2 insertions(+), 3 deletions(-)
Auto-merging doc/api/sqlite.md
[main 74f7d3d073] doc: add documentation for constants
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 23:43:50 2024 +0100
 1 file changed, 29 insertions(+)
Auto-merging doc/api/sqlite.md
[main c77de92d83] doc: add explanation database name
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 23:49:52 2024 +0100
 1 file changed, 3 insertions(+), 1 deletion(-)
[main 4f405293b2] test: remove unused imports test-sqlite.js
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 23:50:14 2024 +0100
 1 file changed, 3 deletions(-)
[main 266d848a22] test: add some extra asserts for changeset and patchset
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 23:53:47 2024 +0100
 1 file changed, 10 insertions(+), 1 deletion(-)
[main d78df860a6] test: create suite for conflict resolution
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 2 23:58:35 2024 +0100
 1 file changed, 66 insertions(+), 64 deletions(-)
[main 6cbc255471] test: fix deepStrictEqual helper
 Author: Bart Louwers <bart@emeel.net>
 Date: Sun Nov 3 00:08:41 2024 +0100
 1 file changed, 7 insertions(+), 1 deletion(-)
[main 27092f6cc6] test: fix lint issues
 Author: Bart Louwers <bart@emeel.net>
 Date: Sun Nov 3 00:10:41 2024 +0100
 1 file changed, 9 insertions(+), 10 deletions(-)
Auto-merging doc/api/sqlite.md
[main 0504b8f830] doc: fix lint issue
 Author: Bart Louwers <bart@emeel.net>
 Date: Sun Nov 3 00:11:11 2024 +0100
 1 file changed, 2 insertions(+), 2 deletions(-)
[main 0aaae4a9c2] test: format import
 Author: Bart Louwers <bart@emeel.net>
 Date: Sun Nov 3 00:13:37 2024 +0100
 1 file changed, 1 insertion(+), 3 deletions(-)
Auto-merging src/node_sqlite.cc
[main 76f79b0164] sqlite: run format
 Author: Bart Louwers <bart@emeel.net>
 Date: Sat Nov 16 18:31:57 2024 +0100
 1 file changed, 3 insertions(+), 2 deletions(-)
Auto-merging src/env_properties.h
Auto-merging src/node_sqlite.cc
[main b464b1f628] sqlite: add filter and onConflict to env_properties.h
 Author: Bart Louwers <bart@emeel.net>
 Date: Sun Nov 17 02:58:21 2024 +0100
 2 files changed, 5 insertions(+), 12 deletions(-)
Auto-merging src/node_sqlite.cc
[main 03979904cc] sqlite: format
 Author: Bart Louwers <bart@emeel.net>
 Date: Sun Nov 17 02:58:48 2024 +0100
 1 file changed, 2 insertions(+), 1 deletion(-)
Auto-merging src/node_sqlite.cc
[main 81c2d95b72] sqlite: simplify string conversion
 Author: Bart Louwers <bart@emeel.net>
 Date: Sun Nov 17 03:09:00 2024 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 31 commits in the PR. Attempting autorebase.
Rebasing (2/62)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
sqlite: add support for SQLite Session Extension
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD ab61aa2858] sqlite: add support for SQLite Session Extension

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 19:17:33 2024 +0100

7 files changed, 766 insertions(+), 18 deletions(-)

Rebasing (3/62)

Rebasing (4/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: address PR comment @RedYetiDev
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD f164d909eb] doc: address PR comment @RedYetiDev

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 19:21:26 2024 +0100

1 file changed, 8 insertions(+), 10 deletions(-)

Rebasing (5/62)

Rebasing (6/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: keep MemoryInfo related stuff together
Co-authored-by: James M Snell <jasnell@gmail.com>

PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 4e69cc24b0] sqlite: keep MemoryInfo related stuff together

Author: Bart Louwers <bart.louwers@gmail.com>

Date: Sat Nov 2 19:24:26 2024 +0100

1 file changed, 1 insertion(+), 1 deletion(-)

Rebasing (7/62)

Rebasing (8/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: use snake_case for local variables
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 3d861ed7b4] sqlite: use snake_case for local variables

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 19:27:58 2024 +0100

1 file changed, 3 insertions(+), 3 deletions(-)

Rebasing (9/62)

Rebasing (10/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: remove unneccesary use of node namespace
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 3b1385756c] sqlite: remove unneccesary use of node namespace

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 19:38:17 2024 +0100

1 file changed, 36 insertions(+), 38 deletions(-)

Rebasing (11/62)

Rebasing (12/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: improve working filter parameter
Co-authored-by: Aviv Keller <redyetidev@gmail.com>

PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 9c4977bb0e] sqlite: improve working filter parameter

Author: Bart Louwers <bart.louwers@gmail.com>

Date: Sat Nov 2 19:53:16 2024 +0100

1 file changed, 1 insertion(+), 3 deletions(-)

Rebasing (13/62)

Rebasing (14/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: add comment about using static functions
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 0d73c270e7] sqlite: add comment about using static functions

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 19:41:34 2024 +0100

1 file changed, 2 insertions(+)

Rebasing (15/62)

Rebasing (16/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: improve wording default value onConflict
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 3a307b27b5] doc: improve wording default value onConflict

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 19:56:08 2024 +0100

1 file changed, 2 insertions(+), 2 deletions(-)

Rebasing (17/62)

Rebasing (18/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: capitalize Default
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD a826cdf2e2] doc: capitalize Default

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 19:57:27 2024 +0100

1 file changed, 2 insertions(+), 2 deletions(-)

Rebasing (19/62)

Rebasing (20/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: improve wording options documentation
Co-authored-by: Aviv Keller <redyetidev@gmail.com>

PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 3306616d83] doc: improve wording options documentation

Author: Bart Louwers <bart.louwers@gmail.com>

Date: Sat Nov 2 19:57:53 2024 +0100

1 file changed, 1 insertion(+), 1 deletion(-)

Rebasing (21/62)

Rebasing (22/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: improve options documentation
Co-authored-by: Aviv Keller <redyetidev@gmail.com>

PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 12517490d1] doc: improve options documentation

Author: Bart Louwers <bart.louwers@gmail.com>

Date: Sat Nov 2 19:58:48 2024 +0100

1 file changed, 1 insertion(+), 1 deletion(-)

Rebasing (23/62)

Rebasing (24/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: improve wording table parameter
Co-authored-by: Aviv Keller <redyetidev@gmail.com>

PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD dd6238e6d1] doc: improve wording table parameter

Author: Bart Louwers <bart.louwers@gmail.com>

Date: Sat Nov 2 19:59:45 2024 +0100

1 file changed, 1 insertion(+), 2 deletions(-)

Rebasing (25/62)

Rebasing (26/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: improve test name
Co-authored-by: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD ed594d0b41] test: improve test name

Author: Bart Louwers <bart.louwers@gmail.com>

Date: Sat Nov 2 20:00:12 2024 +0100

1 file changed, 1 insertion(+), 1 deletion(-)

Rebasing (27/62)

Rebasing (28/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: move SQLITE_ENABLE_SESSION to node.gyp
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 56b793cc9d] sqlite: move SQLITE_ENABLE_SESSION to node.gyp

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 22:01:38 2024 +0100

2 files changed, 1 insertion(+), 2 deletions(-)

Rebasing (29/62)

Rebasing (30/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: split out sqlite session test and fix null proto
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 8b9d886be9] test: split out sqlite session test and fix null proto

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 22:28:23 2024 +0100

2 files changed, 368 insertions(+), 350 deletions(-)

create mode 100644 test/parallel/test-sqlite-session.js

Rebasing (31/62)

Rebasing (32/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite:  use FIXED_ONE_BYTE_STRING instead of NewFromUtf8
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 484ea4b032] sqlite:  use FIXED_ONE_BYTE_STRING instead of NewFromUtf8

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 22:50:35 2024 +0100

1 file changed, 1 insertion(+), 3 deletions(-)

Rebasing (33/62)

Rebasing (34/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: don't use ToLocalChecked
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD f54b870db4] sqlite: don't use ToLocalChecked

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 22:52:16 2024 +0100

1 file changed, 4 insertions(+), 2 deletions(-)

Rebasing (35/62)

Rebasing (36/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: avoid double check
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD e5f4a0a908] sqlite: avoid double check

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 23:18:02 2024 +0100

1 file changed, 2 insertions(+), 3 deletions(-)

Rebasing (37/62)

Rebasing (38/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: add documentation for constants
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 26578dc8d1] doc: add documentation for constants

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 23:43:50 2024 +0100

1 file changed, 29 insertions(+)

Rebasing (39/62)

Rebasing (40/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: add explanation database name
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD f185dddf75] doc: add explanation database name

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 23:49:52 2024 +0100

1 file changed, 3 insertions(+), 1 deletion(-)

Rebasing (41/62)

Rebasing (42/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: remove unused imports test-sqlite.js
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 830164bf7b] test: remove unused imports test-sqlite.js

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 23:50:14 2024 +0100

1 file changed, 3 deletions(-)

Rebasing (43/62)

Rebasing (44/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: add some extra asserts for changeset and patchset
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 5577f4ab47] test: add some extra asserts for changeset and patchset

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 23:53:47 2024 +0100

1 file changed, 10 insertions(+), 1 deletion(-)

Rebasing (45/62)

Rebasing (46/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: create suite for conflict resolution
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 9e0be141d2] test: create suite for conflict resolution

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 2 23:58:35 2024 +0100

1 file changed, 66 insertions(+), 64 deletions(-)

Rebasing (47/62)

Rebasing (48/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: fix deepStrictEqual helper
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD b04c9b3340] test: fix deepStrictEqual helper

Author: Bart Louwers <bart@emeel.net>

Date: Sun Nov 3 00:08:41 2024 +0100

1 file changed, 7 insertions(+), 1 deletion(-)

Rebasing (49/62)

Rebasing (50/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: fix lint issues
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 8ae5de2f28] test: fix lint issues

Author: Bart Louwers <bart@emeel.net>

Date: Sun Nov 3 00:10:41 2024 +0100

1 file changed, 9 insertions(+), 10 deletions(-)

Rebasing (51/62)

Rebasing (52/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

doc: fix lint issue
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 816c04ff3e] doc: fix lint issue

Author: Bart Louwers <bart@emeel.net>

Date: Sun Nov 3 00:11:11 2024 +0100

1 file changed, 2 insertions(+), 2 deletions(-)

Rebasing (53/62)

Rebasing (54/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: format import
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD 6436f168d3] test: format import

Author: Bart Louwers <bart@emeel.net>

Date: Sun Nov 3 00:13:37 2024 +0100

1 file changed, 1 insertion(+), 3 deletions(-)

Rebasing (55/62)

Rebasing (56/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: run format
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD a8d2937001] sqlite: run format

Author: Bart Louwers <bart@emeel.net>

Date: Sat Nov 16 18:31:57 2024 +0100

1 file changed, 3 insertions(+), 2 deletions(-)

Rebasing (57/62)

Rebasing (58/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: add filter and onConflict to env_properties.h
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD a5ae3b8407] sqlite: add filter and onConflict to env_properties.h

Author: Bart Louwers <bart@emeel.net>

Date: Sun Nov 17 02:58:21 2024 +0100

2 files changed, 5 insertions(+), 12 deletions(-)

Rebasing (59/62)

Rebasing (60/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: format
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD d5483f5112] sqlite: format

Author: Bart Louwers <bart@emeel.net>

Date: Sun Nov 17 02:58:48 2024 +0100

1 file changed, 2 insertions(+), 1 deletion(-)

Rebasing (61/62)

Rebasing (62/62)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

sqlite: simplify string conversion
PR-URL: #54181

Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

Reviewed-By: James M Snell <jasnell@gmail.com>

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
[detached HEAD b0cd8bb780] sqlite: simplify string conversion

Author: Bart Louwers <bart@emeel.net>

Date: Sun Nov 17 03:09:00 2024 +0100

1 file changed, 1 insertion(+), 1 deletion(-)

Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/11883643605

[nodejs-github-bot]

Landed in 746b17e

[TheOneTheOnlyJJ]

Congratulations @louwers! This will open up great opportunities for developers!

But now, what are the next steps? There are quite a few functions left in the API with no wrapper, besides the more complicated async stuff.

Should this discussion move to a new issue?

[louwers]

@TheOneTheOnlyJJ Thank you, I hope so. Sure, I have some ideas for follow-up work. What would be your priority? I will create an issue this week.

[TheOneTheOnlyJJ]

@TheOneTheOnlyJJ Thank you, I hope so. Sure, I have some ideas for follow-up work. What would be your priority? I will create an issue this week.

Your ideas are solid. It makes sense to pursue them as you mentioned.

Maybe the rest of the simpler features could be implemented first, as they would expose more API area of the extension
to developers ASAP, while the more complex features would come later, one by one, as they get implemented.
Among these simpler features are functions like sqlite3session_isempty and sqlite3session_memory_used.

But as an even greater priority, I believe we should create a table that maps the C API of the extension to the Node API implemented up to date directly, with "Not implemented" noted where there's no Node equivalent for the native C API yet (functions, constants, etc.).

I believe this would:

• make it easier for maintainers to keep track of which parts of the API are covered and which are not
• make it easier for willing contributors to find what they could contribute with
• potentially make the process of onboarding new contributors & maintainers easier (I am convinced that this PR took some effort on your part, and having this bring more contributors in would take some pressure off)

Also, if included in the official documentation, this table would:

• make it easier to navigate through the Node API for developers who are more accustomed to the C API
• spare developers the frustration of finding out that an API part they need is not yet covered by the Node API