nodejs
sqlite: support `db.loadExtension`
Closes https://github.com/nodejs/node/issues/53898
This PR will implement loadExtension API to align with other SQLite3 JS library like better-sqlite3, node-sqlite3, jsr:@db/sqlite, bun:sqlite
Example Code:
import { load } from 'sqlite-vec'
import { DatabaseSync } from 'node:sqlite'
const db = new DatabaseSync(':memory:', {
allowExtension: true
})
load(db) // db.loadExtension() support
const { sqlite_version, vec_version } = db.prepare(
'select sqlite_version() as sqlite_version, vec_version() as vec_version;'
).get()
console.log(`SQLite version: ${sqlite_version}, vector version ${vec_version}`)
const items = [
[1, [0.1, 0.1, 0.1, 0.1]],
[2, [0.2, 0.2, 0.2, 0.2]],
[3, [0.3, 0.3, 0.3, 0.3]],
[4, [0.4, 0.4, 0.4, 0.4]],
[5, [0.5, 0.5, 0.5, 0.5]]
]
const query = [0.3, 0.3, 0.3, 0.3]
db.exec('CREATE VIRTUAL TABLE vec_items USING vec0(embedding float[4])')
const insertStmt = db.prepare(
'INSERT INTO vec_items(rowid, embedding) VALUES (?, ?)'
)
for (const [id, vector] of items) {
const rowId = BigInt(id)
const embedding = new Uint8Array(Float32Array.from(vector).buffer)
insertStmt.run(rowId, embedding)
}
const rows = db.prepare(
`
SELECT
rowid,
distance
FROM vec_items
WHERE embedding MATCH ?
ORDER BY distance
LIMIT 3
`
).all(new Uint8Array(Float32Array.from(query).buffer))
console.log(rows)
> sqlite-vec /Users/himself65/Code/node/out/Debug/node --experimental-sqlite ./index.mjs
(node:72169) ExperimentalWarning: SQLite is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
SQLite version: 3.46.0, vector version v0.1.1
[
{ rowid: 3, distance: 0 },
{ rowid: 4, distance: 0.19999998807907104 },
{ rowid: 2, distance: 0.20000001788139343 }
]
Found another small issue: https://github.com/nodejs/core-validate-commit/pull/122
Found another small issue: nodejs/core-validate-commit#122
Duplicate of https://github.com/nodejs/core-validate-commit/pull/121
/cc @nodejs/sqlite (no such group?)
anyway I cc @nodejs/tsc since this is a breaking change
Codecov Report
Attention: Patch coverage is 69.44444% with 22 lines in your changes missing coverage. Please review.
Project coverage is 88.52%. Comparing base (
3c2da4b) to head (08280ba). Report is 233 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/node_sqlite.cc | 69.44% | 8 Missing and 14 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #53900 +/- ##
==========================================
+ Coverage 87.99% 88.52% +0.53%
==========================================
Files 656 657 +1
Lines 188999 189929 +930
Branches 35981 36472 +491
==========================================
+ Hits 166301 168133 +1832
+ Misses 15865 14992 -873
+ Partials 6833 6804 -29
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/node_errors.h | 85.00% <ø> (ø) |
|
| src/node_sqlite.h | 70.00% <ø> (ø) |
|
| src/node_sqlite.cc | 80.25% <69.44%> (-0.93%) |
:arrow_down: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
since this is a breaking change
I took a quick look at the code. I'm not sure what makes this a breaking change though.
I do think the TSC needs to decide if they want to support loading arbitrary native code in this manner. It wouldn't be the first way that Node supports loading native code, so maybe it's fine.
I have no idea how to write a test for load extension now.
Is it possible to create a very trivial extension that can be accessed as a test fixture?
I do think the TSC needs to decide if they want to support loading arbitrary native code in this manner.
It definitely must be disabled when the experimental permission model is enabled, just like native add-ons etc.
For the native test, I'm not sure if we should build an SQLite binding from scratch or if we should just copy some community dll/so/lib to fixtures.
We also need a test that tries to load an unexpected file (that is not a sqlite extension). It should throw, not crash the process.
I'm not sure if we should build an SQLite binding from scratch or if we should just copy some community dll/so/lib to fixtures.
IMO we should build from scratch to avoid any potential attack and make update tests fixture easier if we ever need to update them.
I'm not sure if we should build an SQLite binding from scratch or if we should just copy some community dll/so/lib to fixtures.
IMO we should build from scratch to avoid any potential attack and make update tests fixture easier if we ever need to update them.
let me see how to make it
Any chance we could add an optional "entrypoint" argument to .loadExtension(), like .loadExtension(path, [entrypoint])?
It's the 3rd argument to the sqlite3_load_extension() C function, which can be NULL. A lot of extensions will have different "entrypoints" to enable/disable features that may be unsafe/harmful in different environments. It's supported in better-sqlite3 / jsr:@d3/sqlite
Also: instead of allowLoadExtension being a parameter at Database-creation time, I think it should be a configurable method on the DatabaseSync class, which is what jsr:@db/sqlite does, as well as Python's SQLite library, Ruby's SQLite library, and Rust's SQLite library.
For example:
import { DatabaseSync } from 'node:sqlite'
const db = new DatabaseSync(':memory:');
db.enableLoadExtension(true);
db.loadExtension("./my-extension");
db.enableLoadExtension(false);
db.exec('...');
Or instead of a db.enableLoadExtension(true) method, we could follow jsr:@db/sqlite's approach with writeable properties like db.enableLoadExtension = false.
My rationale: Most people will load all extensions immediately after opening a database, then never do it again. Loading extensions can be pretty dangerous, so most people will want to ensure that no more extensions will load after they've loaded the ones they care about. So being able to manually allow/dis-allow extensions can be a plus.
Additionally, if extension loading is every blocked for any reason (Node.js compile-time flag), then you can catch an error thrown during db.enableLoadExtension(True) time instead of when the Database class is constructed.
@himself65 are you still interested in pursuing this? I think it would be useful functionality to have. I think it's relatively close to being able to land once the test fixture issue is figured out.
@himself65 are you still interested in pursuing this? I think it would be useful functionality to have. I think it's relatively close to being able to land once the test fixture issue is figured out.
yeah I will try to rebase first today.
For the test fixtures, I'm not sure how to add a CI which build from source code?
For the test fixtures, I'm not sure how to add a CI which build from source code?
You might be able to leverage https://github.com/nodejs/node/tree/main/test/cctest, https://github.com/nodejs/node/tree/main/test/embedding, https://github.com/nodejs/node/tree/main/test/js-native-api, etc. for inspiration.
There's also the OpenSSL engine tests:
- https://github.com/nodejs/node/tree/main/test/addons/openssl-test-engine
- https://github.com/nodejs/node/tree/main/test/addons/openssl-client-cert-engine
- https://github.com/nodejs/node/tree/main/test/addons/openssl-key-engine
There's one error from main branch https://github.com/nodejs/node/actions/runs/12147198883/job/33872885740 ?
Failed to start CI
⚠ Something was pushed to the Pull Request branch since the last approving review. ✘ Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/12152419142
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
should load extension successfully (2.57859ms)
21:34:26 Error: Could not load module /home/iojs/build/workspace/node-test-commit-aix/nodes/aix72-ppc64/out/Release/libsqlite_extension.so.
21:34:26 System error: No such file or directory
21:34:26 at TestContext.