sqlite: mark as release candidate

[mcollina]

Mark the SQLite module as release candidate and remove the experimental warning.

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.53%. Comparing base (ce2ec3d) to head (c02731d).
⚠️ Report is 28 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61262      +/-   ##
==========================================
- Coverage   88.54%   88.53%   -0.01%     
==========================================
  Files         704      704              
  Lines      208734   208756      +22     
  Branches    40271    40277       +6     
==========================================
+ Hits       184823   184830       +7     
- Misses      15932    15936       +4     
- Partials     7979     7990      +11     

Files with missing lines	Coverage Δ
lib/sqlite.js	100.00% <ø> (ø)

... and 44 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]

LGTM. I still think there should be a definitive decision made about an async API before stabilizing it though because the presence or lack of an async API could influence the sync API.

[louwers]

What does this mean exactly? Are breaking changes still possible?

We still need to flip the default of defensive mode #60217. Created a PR.

[mike-git374]

Could we get feedback on this issue before marking release candidate: db.prepare(sql, options)

[tpoisseau]

The API looks good for a release candidate.

[mcollina]

LGTM. I still think there should be a definitive decision made about an async API before stabilizing it though because the presence or lack of an async API could influence the sync API.

@cjihrig Is there an issue to track this?

I personally don't think we need an Async API for the generic case. However, something like https://github.com/mcollina/sqlite-pool to provide concurrent read transactions might be interesting to support. https://www.sqlite.org/lang_transaction.html

Having said that, the API to support this would need to be significantly different.

[mcollina]

What does this mean exactly? Are breaking changes still possible?

@louwers yes, breaking changes would still be possible but we'd try to avoid them unless there are major usability issues.

[louwers]

I don't know how much weight my vote counts, but I would definitely wait until the async API (#59109) has landed before marking it as a RC.

I personally think DatabaseSync should be renamed to Database, because synchronous should be the default since 9 out of 10 times it will be more performant than using the (future) asynchronous API.

[mcollina]

Moving to stabilizing what we have does not prevent new features to be added. Keeping api experimental forever is not really good for the project.

[cjihrig]

The tracking issue is at #54307.

I personally think DatabaseSync should be renamed to Database, because synchronous should be the default since 9 out of 10 times it will be more performant than using the (future) asynchronous API.

This is the type of thing I meant. The reason the name DatabaseSync was chosen was to follow the naming convention used in other Node core modules. If there is going to be an async API, it would be nice to keep those conventions and make the two APIs as similar as reasonable. If there is not going to be a dedicated async API, then yes, it probably makes sense to just name it Database. Personally, I originally thought we should have an async API, but I've changed my mind about that recently.

[geeksilva97]

LGTM. I still think there should be a definitive decision made about an async API before stabilizing it though because the presence or lack of an async API could influence the sync API.

@cjihrig Is there an issue to track this?

I personally don't think we need an Async API for the generic case. However, something like https://github.com/mcollina/sqlite-pool to provide concurrent read transactions might be interesting to support. https://www.sqlite.org/lang_transaction.html

Having said that, the API to support this would need to be significantly different.

The issue is: #54307.

I took longer than I expected, but I've been working on it. I love the reference to the sqlite-pool, it's something I added due to the nature of sqlite with concurrent operations.

I should, finally, have a PR for review by the end of this week

[mike-git374]

I personally think DatabaseSync should be renamed to Database, because synchronous should be the default since 9 out of 10 times it will be more performant than using the (future) asynchronous API.

The reason the name DatabaseSync was chosen was to follow the naming convention used in other Node core modules.

import { Database, /* DatabaseAsync */ } from 'node:sqlite';

I think SQLite is different, the sync version is the primary and recommended use case. If you name the Async version Database and the Sync version DatabaseSync you could give the impression that the Async version is normal or on even ground or better than Sync, whereas actually Async SQLite is a special case that should not be the default. In this situation Database for Sync works and a potential future DatabaseAsync could be used if it's added at all, as some have suggested to not add it.

[cjihrig]

I think SQLite is different, the sync version is the primary and recommended use case.

I think trying to maintain consistency within core is a good thing. And, with all due respect, that is just your opinion. While I do agree with you, I have seen plenty of people complain about node:sqlite blocking the event loop.

[mike-git374]

Ok I was just taking into account your recent post:

Personally, I originally thought we should have an async API, but I've changed my mind about that recently.

If there is not going to be a dedicated async API, then yes, it probably makes sense to just name it Database

[louwers]

I think trying to maintain consistency within core is a good thing. And, with all due respect, that is just your opinion.

We could test it. I think for a lot of queries, the serialization overhead to and from a worker thread is larger than running the query in the main thread. Guiding people to use the recommended API (in my opinion) is more important than consistency. Is it really more consistent though? Is there another class with a Sync postfix?

I am not a fan of the API where you have separate DatabaseAync, StatementAsync, SQLTagStoreAsync classes (by the way, why is the existing SQLTagStore not SQLTagStoreSync?). I don't know if a StatementAsync class even makes sense (@geeksilva97 probably thought about this) because it cannot have any correspondence to a SQLite prepared statement, because those can only be used on one thread (in multi-thread mode), and presumably the thread it gets run on is only determined when it is run. What does DatabaseAsync.prepare() do?

Maybe we could rename DatabaseSync to Database and and allow retrieving a handle to a database pool with an API that fits async better:

const { Database } from 'node:sqlite';

const db = new Database('...', {
  threadingMode: 'multiThread'
});
// will fail if threadingMode is 'singleThread'
const dbPool = db.pool; // DatabasePool instance

await db.pool.exec('...');  // async
db.exec('...');  // sync

Another advantage would be that is that it's easier to use both sync and async queries from the same DB handle.

[cjihrig]

Is it really more consistent though? Is there another class with a Sync postfix?

I'm not sure if there are any classes, but IMO it's definitely more consistent with, for example, the fs, child_process, and zlib APIs which use Sync to indicate synchronous and no suffix to indicate asynchronous. The reason I added the Sync on the class name is because I don't think we can design an efficient database connection that does both sync and async work. We could add Sync to the individual method names, but that seems worse.

I'll stop commenting on this discussion though.

[louwers]

Let's move the discussion to #54307

[Waldenesque]

I don't know if everyone noticed, but sqlite3 (no release since January 2024 with Nov '23 SQLite before JSONB) recently marked itself "DEPRECATED" stating: "This repository is currently unmaintained. We will not update any of its issues or pull requests." In other words the de facto async API is now officially abandoned.

Also better-sqlite3 is not a Node-API addon and switching to N-API would require a full rewrite, so @JoshuaWise and @mceachen must periodically check compat with internal C++ APIs and update supported Node versions and Electron builds. Thus the de facto sync API is maintained but needs extra fuss which over the long run isn't ideal.

Meanwhile Deno is directly copying Node's new built-in API for their own first-party SQLite support (and Deno's older third-party SQLite drivers are largely inactive, so they have incentive to do a quality implementation). Two major JS runtimes supporting the same code makes this API more appealing.

My suspicion is this API will eventually become the new de facto standard, so now is the last chance before "coulda woulda shouldas." Someone noticed better-sqlite3 enables defensive mode by default so @louwers had to quickly create a similar PR here. I wonder about other sanity checks or lingering details, for instance will tagged template support be manually backported to Node 22? I'm not pushing either way as it's someone else's time, but it's worth noting if that feature will work in all versions of Node with the API before May 2027.

Most importantly I have the same hesitation as the creator of this API: it feels premature to stabilize without at least a preliminary decision whether or not an async API is intended. A PR has been open since July including a recent question if some of the sync API should be renamed. With the legacy async API now abandoned (despite over a million weekly downloads), it seems worth trying to reach a tentative consensus whether or not a built-in async API is at least planned. It doesn't matter how long an async implementation might take. The concern is what @cjihrig said: "the presence or lack of an async API could influence the sync API."

[Waldenesque]

It occurred to me just hand-waving is counterproductive as that invites bikeshedding which I'd rather not encourage. If this post is folly moderators can hide it, but I'll try to outline how this appears to me as a frame of reference. Basically I see two scenarios here.

The Someday Scenario: Someday Node will add a built-in async API alongside the sync API.

The Never Scenario: Node will never need an explicit async API, the sync API is enough.

At a high level, I'd guess that decision boils down to something pointed out by @cjihrig. The suggested approach from better-sqlite3 seems to be manually writing plumbing code to spawn workers exchanging messages with a main thread coordinator. If expecting users to write their own plumbing code is acceptable, then a separate explicit async API is unnecessary, which is the Never Scenario.

The alternative is some kind of wrapper interface that automates and abstracts away some of that lower level plumbing, for instance error handling for crashed or hung workers, presenting a simpler and more consistent usage story for users, hopefully encouraging more predictable and reliable usage patterns across the ecosystem.

(Relevant side note! JavaScript and Python are both default newbie languages, yet Python is more common in academic CS settings, plus having batteries included helps steer novice coders. Thus JavaScript stands alone in sheer number of unguided amateurs haphazardly stumbling into Turing tarpits. Furthermore it's easier for newbies to start exploring SQLite, especially now that it can be used instantly by default in all three major JS runtimes, versus setting up Postgres or MySQL or maybe even MongoDB. Point being, there's an argument to be made that in our wonderful world of JavaScript, it's worth striving harder than usual for a pit of success.)

I suspect perceived complexity of an async API depends on the scope of anticipated use cases, which boils loosely down to two schools of thought, the familiar or the fancy. The first is: one or more discrete SQLite instances, with each exclusively accessing one database at a time, and there happen to be some slow queries. The second is "everything else." If you target the first audience, and explicitly document "everything else" as out of scope, I'll bet you can actually ship something, and most folks will be happy as that's all they need. A built-in async API would be helpful chiefly for easing usability in boring everyday use cases that happen to involve long-running queries. Advise experts with more challenging situational needs (simultaneous multiple writers, cycling umpteen databases, exotic query flow, whatever) to just ignore the async API, and devise their own bespoke plumbing using the sync API. If you try chasing a more grandiose async story, unless miraculously some indefatigable volunteer jumps on it as a wild romantic passion project, I'll bet that turns into a "Never Never Scenario."

If it's considered worthwhile to pursue a pragmatically feasible async API, handling routine plumbing for mundane use cases, this is the Someday Scenario. In closing I'll quote @everett1992 who perfectly summed up the Someday Scenario sticking point: "I don't think async support should be required to be implemented before the sync API is stable. We just need enough of the async API sketched out to be confident that it meshes with the sync model."

Hopefully this encourages clarity and pragmatic focus. If not, moderators please hide this comment.