From f87234eed4925df5690da4c128fe60c365f0b9ad Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 5 Sep 2025 09:36:50 -0700 Subject: [PATCH] feat: cache parameter bindings --- .nvmrc | 2 +- bench/insert-blob.bench.js | 18 +++++++++- bench/insert.bench.js | 18 +++++++++- bench/select.bench.js | 18 +++++++++- lib/index.ts | 72 +++++++++++++++++++++++++++++++------- src/addon.cc | 50 ++++++++++++-------------- test/memory.test.ts | 15 ++++---- 7 files changed, 141 insertions(+), 52 deletions(-) diff --git a/.nvmrc b/.nvmrc index 0254b1e..91d5f6f 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -20.18.2 +22.18.0 diff --git a/bench/insert-blob.bench.js b/bench/insert-blob.bench.js index cf4cfdc..6abb636 100644 --- a/bench/insert-blob.bench.js +++ b/bench/insert-blob.bench.js @@ -2,7 +2,8 @@ import { Buffer } from 'node:buffer'; import { bench, describe } from 'vitest'; import BDatabase from '@signalapp/better-sqlite3'; -import Database from '../lib/index.js'; +import { DatabaseSync as NDatabase } from 'node:sqlite'; +import Database from '../dist/index.mjs'; const PREPARE = ` CREATE TABLE t ( @@ -21,12 +22,15 @@ const DELETE = 'DELETE FROM t'; describe('INSERT INTO t', () => { const sdb = new Database(':memory:', { cacheStatements: true }); const bdb = new BDatabase(':memory:'); + const ndb = new NDatabase(':memory:'); sdb.exec(PREPARE); bdb.exec(PREPARE); + ndb.exec(PREPARE); const sinsert = sdb.prepare(INSERT); const binsert = bdb.prepare(INSERT); + const ninsert = ndb.prepare(INSERT); bench( '@signalapp/sqlcipher', @@ -51,4 +55,16 @@ describe('INSERT INTO t', () => { }, }, ); + + bench( + 'node:sqlite', + () => { + ninsert.run({ b: BLOB }); + }, + { + teardown: () => { + ndb.exec(DELETE); + }, + }, + ); }); diff --git a/bench/insert.bench.js b/bench/insert.bench.js index 2377f5d..bc555b0 100644 --- a/bench/insert.bench.js +++ b/bench/insert.bench.js @@ -1,7 +1,8 @@ import { bench, describe } from 'vitest'; import BDatabase from '@signalapp/better-sqlite3'; -import Database from '../lib/index.js'; +import { DatabaseSync as NDatabase } from 'node:sqlite'; +import Database from '../dist/index.mjs'; const PREPARE = ` CREATE TABLE t ( @@ -24,12 +25,15 @@ const DELETE = 'DELETE FROM t'; describe('INSERT INTO t', () => { const sdb = new Database(':memory:', { cacheStatements: true }); const bdb = new BDatabase(':memory:'); + const ndb = new NDatabase(':memory:'); sdb.exec(PREPARE); bdb.exec(PREPARE); + ndb.exec(PREPARE); const sinsert = sdb.prepare(INSERT); const binsert = bdb.prepare(INSERT); + const ninsert = ndb.prepare(INSERT); bench( '@signalapp/sqlcipher', @@ -54,4 +58,16 @@ describe('INSERT INTO t', () => { }, }, ); + + bench( + 'node:sqlite', + () => { + ninsert.run({ a1: 1, a2: 2, a3: 3, b1: 'b1', b2: 'b2', b3: 'b3' }); + }, + { + teardown: () => { + ndb.exec(DELETE); + }, + }, + ); }); diff --git a/bench/select.bench.js b/bench/select.bench.js index 9bc5291..94c43a3 100644 --- a/bench/select.bench.js +++ b/bench/select.bench.js @@ -1,7 +1,8 @@ import { bench, describe } from 'vitest'; import BDatabase from '@signalapp/better-sqlite3'; -import Database from '../lib/index.js'; +import { DatabaseSync as NDatabase } from 'node:sqlite'; +import Database from '../dist/index.mjs'; const PREPARE = ` CREATE TABLE t ( @@ -36,12 +37,15 @@ const SELECT = 'SELECT * FROM t LIMIT 1000'; describe('SELECT * FROM t', () => { const sdb = new Database(':memory:', { cacheStatements: true }); const bdb = new BDatabase(':memory:'); + const ndb = new NDatabase(':memory:'); sdb.exec(PREPARE); bdb.exec(PREPARE); + ndb.exec(PREPARE); const sinsert = sdb.prepare(INSERT); const binsert = bdb.prepare(INSERT); + const ninsert = ndb.prepare(INSERT); sdb.transaction(() => { for (const value of VALUES) { @@ -55,6 +59,12 @@ describe('SELECT * FROM t', () => { } })(); + ndb.exec('BEGIN'); + for (const value of VALUES) { + ninsert.run(value); + } + ndb.exec('COMMIT'); + const sselect = sdb.prepare(SELECT); const bselect = bdb.prepare(SELECT); @@ -65,4 +75,10 @@ describe('SELECT * FROM t', () => { bench('@signalapp/better-sqlite', () => { bselect.all(); }); + + bench('node:sqlite', () => { + // Node.js seems to finalize the statement after `.all()` + const nselect = ndb.prepare(SELECT); + nselect.all(); + }); }); diff --git a/lib/index.ts b/lib/index.ts index e6b1fa1..7821adf 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -27,15 +27,16 @@ const addon = bindings<{ persistent: boolean, pluck: boolean, bigint: boolean, + paramNames: Array, ): NativeStatement; statementRun( stmt: NativeStatement, - params: StatementParameters | undefined, + params: NativeParameters | undefined, result: [number, number], ): void; statementStep( stmt: NativeStatement, - params: StatementParameters | null | undefined, + params: NativeParameters | null | undefined, cache: Array> | undefined, isGet: boolean, ): Array>; @@ -85,11 +86,15 @@ export type StatementOptions = Readonly<{ bigint?: true; }>; +export type NativeParameters = ReadonlyArray< + SqliteValue +>; + /** * Parameters accepted by `.run()`/`.get()`/`.all()` methods of the statement. */ export type StatementParameters = - | ReadonlyArray> + | NativeParameters | Readonly>>; /** @@ -119,6 +124,9 @@ class Statement { #cache: Array> | undefined; #createRow: undefined | ((result: unknown) => RowType); + #translateParams: ( + params: StatementParameters, + ) => NativeParameters; #native: NativeStatement | undefined; #onClose: (() => void) | undefined; @@ -131,14 +139,47 @@ class Statement { ) { this.#needsTranslation = persistent === true && !pluck; + const paramNames = new Array(); + this.#native = addon.statementNew( db, query, persistent === true, pluck === true, bigint === true, + paramNames, ); + const isArrayParams = paramNames.every((name) => name === null); + const isObjectParams = + !isArrayParams && paramNames.every((name) => typeof name === 'string'); + + if (!isArrayParams && !isObjectParams) { + throw new TypeError('Cannot mix named and anonymous params in query'); + } + + if (isArrayParams) { + this.#translateParams = (params) => { + if (!Array.isArray(params)) { + throw new TypeError('Query requires an array of anonymous params'); + } + return params; + }; + } else { + this.#translateParams = runInThisContext(` + (function translateParams(params) { + if (Array.isArray(params)) { + throw new TypeError('Query requires an object of named params'); + } + return [ + ${paramNames + .map((name) => `params[${JSON.stringify(name)}]`) + .join(',\n')} + ]; + }) + `); + } + this.#onClose = onClose; } @@ -154,8 +195,8 @@ class Statement { throw new Error('Statement closed'); } const result: [number, number] = [0, 0]; - this.#checkParams(params); - addon.statementRun(this.#native, params, result); + const nativeParams = this.#checkParams(params); + addon.statementRun(this.#native, nativeParams, result); return { changes: result[0], lastInsertRowid: result[1] }; } @@ -174,8 +215,13 @@ class Statement { if (this.#native === undefined) { throw new Error('Statement closed'); } - this.#checkParams(params); - const result = addon.statementStep(this.#native, params, this.#cache, true); + const nativeParams = this.#checkParams(params); + const result = addon.statementStep( + this.#native, + nativeParams, + this.#cache, + true, + ); if (result === undefined) { return undefined; } @@ -202,9 +248,8 @@ class Statement { throw new Error('Statement closed'); } const result = []; - this.#checkParams(params); - let singleUseParams: StatementParameters | undefined | null = - params; + const nativeParams = this.#checkParams(params); + let singleUseParams: typeof nativeParams | undefined | null = nativeParams; while (true) { const single = addon.statementStep( this.#native, @@ -282,9 +327,11 @@ class Statement { } /** @internal */ - #checkParams(params: StatementParameters | undefined): void { + #checkParams( + params: StatementParameters | undefined, + ): NativeParameters | undefined { if (params === undefined) { - return; + return undefined; } if (typeof params !== 'object') { throw new TypeError('Params must be either object or array'); @@ -292,6 +339,7 @@ class Statement { if (params === null) { throw new TypeError('Params cannot be null'); } + return this.#translateParams(params); } } diff --git a/src/addon.cc b/src/addon.cc index ed734c0..3f3e4bd 100644 --- a/src/addon.cc +++ b/src/addon.cc @@ -406,12 +406,14 @@ Napi::Value Statement::New(const Napi::CallbackInfo& info) { auto is_persistent = info[2].As(); auto is_pluck = info[3].As(); auto is_bigint = info[4].As(); + auto param_names = info[5].As(); assert(db_external.IsExternal()); assert(query.IsString()); assert(is_persistent.IsBoolean()); assert(is_pluck.IsBoolean()); assert(is_bigint.IsBoolean()); + assert(param_names.IsArray()); auto db = db_external.Data(); @@ -440,6 +442,18 @@ Napi::Value Statement::New(const Napi::CallbackInfo& info) { auto stmt = new Statement(db, db_external, handle, is_persistent, is_pluck, is_bigint); + int key_count = sqlite3_bind_parameter_count(handle); + + for (int i = 1; i <= key_count; i++) { + auto name = sqlite3_bind_parameter_name(handle, i); + if (name == nullptr) { + param_names[i - 1] = env.Null(); + } else { + // Skip "$" + param_names[i - 1] = name + 1; + } + } + return Napi::External::New( env, stmt, [](Napi::Env env, Statement* stmt) { delete stmt; }); } @@ -733,36 +747,18 @@ bool Statement::BindParams(Napi::Env env, Napi::Value params) { for (int i = 1; i <= list_len; i++) { auto name = sqlite3_bind_parameter_name(handle_, i); - if (name != nullptr) { - NAPI_THROW(FormatError(env, "Unexpected named param %s at %d", name, i), - false); - } auto error = BindParam(env, i, list[i - 1]); if (error != nullptr) { - NAPI_THROW( - FormatError(env, "Failed to bind param %d, error %s", i, error), - false); - } - } - } else { - auto obj = params.As(); - - for (int i = 1; i <= key_count; i++) { - auto name = sqlite3_bind_parameter_name(handle_, i); - if (name == nullptr) { - NAPI_THROW(FormatError(env, "Unexpected anonymous param at %d", i), - false); - } - - // Skip "$" - name = name + 1; - auto value = obj[name]; - auto error = BindParam(env, i, value); - if (error != nullptr) { - NAPI_THROW( - FormatError(env, "Failed to bind param %s, error %s", name, error), - false); + if (name == nullptr) { + NAPI_THROW( + FormatError(env, "Failed to bind param %d, error %s", i, error), + false); + } else { + NAPI_THROW(FormatError(env, "Failed to bind param %s, error %s", + name + 1, error), + false); + } } } } diff --git a/test/memory.test.ts b/test/memory.test.ts index cd1bfdb..4d4e83c 100644 --- a/test/memory.test.ts +++ b/test/memory.test.ts @@ -212,12 +212,16 @@ describe('list parameters', () => { test('object parameters', () => { const stmt = db.prepare('SELECT * FROM t WHERE a > ?'); - expect(() => stmt.get({})).toThrowError('Unexpected anonymous param at 1'); + expect(() => stmt.get({})).toThrowError( + 'Query requires an array of anonymous params', + ); }); test('against named parameters', () => { const stmt = db.prepare('SELECT * FROM t WHERE a > $a'); - expect(() => stmt.get([2])).toThrowError('Unexpected named param $a at 1'); + expect(() => stmt.get([2])).toThrowError( + 'Query requires an object of named params', + ); }); }); @@ -239,13 +243,6 @@ describe('object parameters', () => { const stmt = db.prepare('SELECT * FROM t WHERE a > $a'); expect(() => stmt.get()).toThrowError('Expected 1 parameters, got 0'); }); - - test('against anonymous parameters', () => { - const stmt = db.prepare('SELECT * FROM t WHERE a > ?'); - expect(() => stmt.get({ a: 1 })).toThrowError( - 'Unexpected anonymous param at 1', - ); - }); }); describe('tail', () => {