Review requested:

• @nodejs/sqlite

Codecov Report

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

@@            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     

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.

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.

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.

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.

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

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.

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.

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.

Let's move the discussion to #54307

@Waldenesque I think you offer a valuable viewpoint. The async API not being ready should not be a blocker for stabilizing the SQLite API, but we should at least agree on what the extension point will be when and if an async API is added. For example if we should have a unified Database class for async and sync operations, or introduce a separate class.

The stability index definition suggests:

1.2 - Release candidate. Experimental features at this stage are hopefully ready to become stable. No further breaking changes are anticipated but may still occur in response to user feedback or the features' underlying specification development. We encourage user testing and feedback so that we can know that this feature is ready to be marked as stable.

So if the worry is that "there will be changes", 1.2 does not forbid that and already warns about it. Personally my understanding of the difference between 1.1. and 1.2 is "how certain you are about the likelihood of changes - >x% or < x%?" (different people might have different x, I'd say 20% is my personal threshold). If the worry is that it will encourage usage and make changes more difficult, perhaps it's worth gauging how many users are already using to make it difficult already.

Note that breaking changes even after a feature reaches stability are still possible. Just that they will have to be semver-major (in experimental stages, that's not a hard requirement, but can still be done out of caution).

SQLite is fundamentally a sync API. Pretending that it’s async is a footgun-rich facade

I don't think pretending it will be async is the goal.

Offense certainly wasn’t intended.

My guess is that if you asked a normal node user, (especially if they’ve interacted with if MySQL or Postgres) if an async call could block another async call, I suspect that they would be surprised. Any async api will require careful verbiage to describe the footguns.

I agree keeping Sync in the name is good to label things that can block. I would remove the redundant statement.set* functions now that db.prepare(sql, options) was added to clean up the API, unless there is some reason to have multiple ways to do the same thing, usually it's better to have a unified API

I removed my earlier comments, which I now think are distractions. Colin's answer to my question stands by itself, so there's no point keeping my chatter around. (I was never eager to stick my nose into this in the first place, I did it reluctantly.)

After some ruminations I've come around to feeling a warm fuzzy about the sync API moving toward stabilization as it is. If anyone is still on the other side, it might be useful for them to know why, and I was writing something. However I just noticed @louwers seems to be coming around too, so for now I'll skip it. Maybe everybody is finding warm fuzzies.

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

Would be nice to clean up the API and remove the 4 now redundant functions
statement.setAllowBareNamedParameters(enabled)
statement.setAllowUnknownNamedParameters(enabled)
statement.setReturnArrays(enabled)
statement.setReadBigInts(enabled)

Can we get confirmation from reviewers you want to keep these before marking as release candidate. Would be nice to have everyone setting statement options the same way with database.prepare(sql[, options])

@mike-git374 why are they redundant? Is there a tracking issue for that removal?

They are not completely redundant. The existing methods allow you to change the behavior of an existing prepared statement, which database.prepare() does not.

This seems ready to merge. Please land it.

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @mcollina.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

Landed in c9acf34

commit c9acf345922bd758fbb3f16ee6256aa165260219 (upstream/main)
Author: Matteo Collina <...>
Date:   Sat Feb 21 17:30:23 2026 +0100

    sqlite: mark as release candidate

    PR-URL: REPLACEME
    PR-URL: https://github.com/nodejs/node/pull/61262
    Reviewed-By: ...

Is this going to break branch-diff?

They are not completely redundant. The existing methods allow you to change the behavior of an existing prepared statement, which database.prepare() does not.

Although the C++ set* methods do allow mutation after construction, I couldn't think of a use case that prepare(sql, options) doesn't already handle.

Prepared statements, as far as SQLite is concerned, are immutable constructs. If we adopt that as well it reduces both the API complexity and the potential for surprising behavior.

These options are purely Node.js-side marshaling flags (not SQLite state), and since returnArrays and readBigInts are read per-row during iteration, mid-iteration mutation would silently produce mixed-format results. I can't imagine a scenario where someone would want this behavior.

(I think node:sqlite would be better if we dropped those functions as @mike-git374 suggested)

FYI statement.set* functions were originally removed in PR #61311 but @Renegade334 requested it to be a separate PR

I agree that using the options on prepare() is generally a better option. However, the other APIs have existed for a long time now and people are using them in the wild. It looks like #61311 only exists on v25. We shouldn't aggressively break users just because an API is still experimental. There isn't any harm in having both options - at least until the prepare() options exist on all supported branches.

EDIT: It's also worth noting that chaining calls on the statement object seems to be more in line with how other SQLite APIs work. Neither better-sqlite3 nor bun:sqlite appear to have documented options passed to prepare(). This has the potential to cause problems for any libraries that try to create a uniform API on top of these different SQLite libraries.