Skip to content

Conversation

@cjihrig
Copy link
Contributor

@cjihrigcjihrig commented Jul 7, 2024

#53264 has been open for over a month with no objections, so I am opening this PR with an initial node:sqlite module. There is other functionality that could potentially be exposed in the future, but I believe this is enough for an experimental MVP.

Fixes: #53264

Summary: Node.js now includes a built-in sqlite module (require('node:sqlite')) that becomes available when using the --experimental-sqlite flag

The following example shows the basic usage of the node:sqlite module to open
an in-memory database, write data to the database, and then read the data back.

import{DatabaseSync}from'node:sqlite';constdatabase=newDatabaseSync(':memory:');// Execute SQL statements from strings.database.exec(` CREATE TABLE data( key INTEGER PRIMARY KEY, value TEXT ) STRICT`);// Create a prepared statement to insert data into the database.constinsert=database.prepare('INSERT INTO data (key, value) VALUES (?, ?)');// Execute the prepared statement with bound values.insert.run(1,'hello');insert.run(2,'world');// Create a prepared statement to read data from the database.constquery=database.prepare('SELECT * FROM data ORDER BY key');// Execute the prepared statement and log the result set.console.log(query.all());// Prints: [{key: 1, value: 'hello' },{key: 2, value: 'world' } ]

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 7, 2024
@cjihrigcjihrig requested a review from benjamingrJuly 7, 2024 16:57
@avivkelleravivkeller added the experimental Issues and PRs related to experimental features. label Jul 7, 2024
Copy link
Member

@targostargos left a comment

Choose a reason for hiding this comment

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

I can't review the C++ code, but API and tests LGTM for a MVP.

Copy link
Member

@benjamingrbenjamingr left a comment

Choose a reason for hiding this comment

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

Yay, happy to see this come to fruition. Let's land as experimental and iterate 🎉


Constructs a new `SQLiteDatabaseSync` instance.

### `database.close()`
Copy link
Member

Choose a reason for hiding this comment

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

It would be nifty to also support Symbol.dispose here but that can be in a follow up PR.

@benjamingr
Copy link
Member

This is a lot more feature complete and ready than I was expecting, good job Colin!

H4ad
H4ad approved these changes Jul 7, 2024
@avivkeller
Copy link
Member

This is adding a new feature, so shouldn't be semver-minor? (and with this kind of change, notable-change as well?)

Copy link
Contributor

@ShogunPandaShogunPanda left a comment

Choose a reason for hiding this comment

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

Love it! Amazing job @cjihrig

Copy link
Member

@mcollinamcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollinamcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2024
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2024
@nodejs-github-bot
Copy link
Collaborator

@targostargos added semver-minor PRs that contain new features and should be released in the next minor version. experimental Issues and PRs related to experimental features. and removed experimental Issues and PRs related to experimental features. labels Jul 8, 2024
Qard
Qard approved these changes Jul 8, 2024
Copy link
Member

@QardQard left a comment

Choose a reason for hiding this comment

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

Should we be concerned at all about the performance impact of people using these sync APIs within requests and blocking the event loop? With sync being the only option here I feel like the risk is increased.

If the concern is simplicity, I feel like only async would be better than only sync. Especially given top-level await. What reason is there to favour sync over async here?

I won't block on that concern, and in fact I think the work done so far is great. I would be happy landing what's here now. I just wanted to share my concern with going for sync first rather than async first.

@benjamingr
Copy link
Member

Should we be concerned at all about the performance impact of people using these sync APIs within requests and blocking the event loop? With sync being the only option here I feel like the risk is increased.

SQLite runs in process and is often CPU bound, we do have async APIs for some CPU intensive stuff (like in crypto) but I suspect for most people this API is the better one performance wise (that said, in some cases an async API is preferable and we should expose that as well)

ehsankhfr pushed a commit to ehsankhfr/node that referenced this pull request Jul 18, 2024
Notable changes: http: * (SEMVER-MINOR) expose websockets (Natalia Venditto) nodejs#53721 lib: * (SEMVER-MINOR) add `node:sqlite` module (Colin Ihrig) nodejs#53752 module: * add `__esModule` to `require()`'d ESM (Joyee Cheung) nodejs#52166 path: * (SEMVER-MINOR) add `matchesGlob` method (Aviv Keller) nodejs#52881 process: * (SEMVER-MINOR) port on-exit-leak-free to core (Vinicius Lourenço) nodejs#53239 stream: * (SEMVER-MINOR) pipeline wait for close before calling the callback (jakecastelli) nodejs#53462 test_runner: * support glob matching coverage files (Aviv Keller) nodejs#53553 worker: * (SEMVER-MINOR) add `postMessageToThread` (Paolo Insogna) nodejs#53682 PR-URL: nodejs#53826
@thomasdao
Copy link

I'm really grateful that Sqlite is natively supported in NodeJS and want to thank @cjihrig and everyone involved for working on this feature. Thanks for choosing Sqlite - it's fast and much more reliable than most alternatives. This feature would be super useful for Electron users - currently most Sqlite bindings don't work well for Electron apps, and having native support for Sqlite would make it easier to interact with the database.

@louwers
Copy link
Contributor

This is awesome! Is support for official SQLite extensions something that is being considered? In particular I am interested in the Session Extension.

@cjihrig
Copy link
ContributorAuthor

cjihrig commented Jul 24, 2024

@louwers it hasn't been discussed AFAIK, but there is a PR for it, so it's not out of the question: #53900.

EDIT: Oh, that one is built in already, we would just need to support enabling it.

@louwers
Copy link
Contributor

louwers commented Jul 29, 2024

@cjihrig Actually the Session Extension is not a runtime loadable extension. It needs to be compiled into SQLite.

To be able to use it with Node.js you would need to wrap its C API: https://www.sqlite.org/session/intro.html

@TheOneTheOnlyJJ
Copy link

TheOneTheOnlyJJ commented Jul 30, 2024

To be able to use it with Node.js you would need to wrap its C API: https://www.sqlite.org/session/intro.html

+1 for this.

Supporting all of the sqlite3 amalgamation extensions should be a strong goal for Node.js (through wrapping the C APIs of each).
While at first glance they may not look relevant, these extensions are very powerful & vital for the projects that make use of them.

Maybe a new issue concerning this topic specifically should be opened?

@louwers
Copy link
Contributor

louwers commented Aug 2, 2024

@cjihrig@TheOneTheOnlyJJ I opened a PR so we can have some discussion about the API (among other things): #54181

richardlau pushed a commit to nodejs/core-validate-commit that referenced this pull request Aug 5, 2024
@targostargos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 21, 2024
Comment on lines +633 to +639
t.assert.throws(() =>{
const stmt = db.prepare('INSERT INTO types (key, val) VALUES ($k, $v)');
stmt.run({$k: 1, $unknown: 1 });
},{
code: 'ERR_INVALID_STATE',
message: /Unknown named parameter '\$unknown'/,
});

Choose a reason for hiding this comment

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

@cjihrig Thanks for your work!

Regarding this case, I am a bit skeptical because better-sqlite3 allows for more named parameters to be passed than those actually used in the SQL. This can be useful when you build your SQL with conditions in a literal string, for example.

@cjihrigcjihrig mentioned this pull request Nov 14, 2024
cjihrig added a commit to cjihrig/node that referenced this pull request Mar 16, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: nodejs#53752 (comment)
nodejs-github-bot pushed a commit that referenced this pull request Mar 18, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
aduh95 pushed a commit that referenced this pull request Mar 23, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 1, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 1, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit to RafaelGSS/node that referenced this pull request Apr 8, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: nodejs#53752 (comment) PR-URL: nodejs#57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 14, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 14, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
aduh95 pushed a commit that referenced this pull request Apr 14, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
aduh95 pushed a commit that referenced this pull request Apr 14, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
aduh95 pushed a commit that referenced this pull request Apr 15, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 16, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 17, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
This commit adds support for the explicit resource management proposal to the sqlite module. Refs: #53752 (comment) PR-URL: #57506 Reviewed-By: Zeyu "Alex" Yang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Edy Silva <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.dont-land-on-v20.xPRs that should not land on the v20.x-staging branch and should not be released in v20.x.experimentalIssues and PRs related to experimental features.lib / srcIssues and PRs related to general changes in the lib or src directory.notable-changePRs with changes that should be highlighted in changelogs.semver-minorPRs that contain new features and should be released in the next minor version.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose SQLite?

19 participants

@cjihrig@nodejs-github-bot@benjamingr@avivkeller@Qard@nyteshade@khaosdoctor@tlhunter@RafaelGSS@aduh95@targos@karlhorky@karimfromjordan@bholmesdev@mpnkhan@thomasdao@louwers@TheOneTheOnlyJJ@mcollina