From 2e66b8cd94665cca68ea246302301ce4952ab6f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 19 Feb 2026 10:53:46 +0000 Subject: [PATCH 01/15] sqlite: extract `DatabaseSync` tmpl into separate function The `node:sqlite` module `Initialize` would get quite large if both of the async and sync database templates were embedded. Therefore move the template creation into a seperate function. I've avoided the `GetConstructorTemplate` pattern, because it seems to imply exposing the template via `PER_ISOLATE_TEMPLATE_PROPERTIES` which is unnecessary in our case. --- src/node_sqlite.cc | 141 +++++++++++++++++++++++---------------------- 1 file changed, 72 insertions(+), 69 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 12b8fdcdebe04b..db4e37a01aa591 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -130,6 +130,23 @@ using v8::Value; } while (0) namespace { + +inline void SetSideEffectFreeGetter(Isolate* isolate, + Local class_template, + Local name, + FunctionCallback fn) { + Local getter = + FunctionTemplate::New(isolate, + fn, + Local(), + v8::Signature::New(isolate, class_template), + /* length */ 0, + ConstructorBehavior::kThrow, + SideEffectType::kHasNoSideEffect); + class_template->InstanceTemplate()->SetAccessorProperty( + name, getter, Local(), DontDelete); +} + Local getLazyIterTemplate(Environment* env) { auto iter_template = env->iter_template(); if (iter_template.IsEmpty()) { @@ -238,8 +255,6 @@ void JSValueToSQLiteResult(Isolate* isolate, } } -class DatabaseSync; - inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, DatabaseSync* db) { if (db->ShouldIgnoreSQLiteError()) { db->SetIgnoreNextSQLiteError(false); @@ -906,6 +921,56 @@ void DatabaseSync::MemoryInfo(MemoryTracker* tracker) const { "open_config", sizeof(open_config_), "DatabaseOpenConfiguration"); } +namespace { +v8::Local CreateDatabaseSyncConstructorTemplate( + Environment* env) { + Isolate* isolate = env->isolate(); + + Local tmpl = + NewFunctionTemplate(isolate, DatabaseSync::New); + tmpl->InstanceTemplate()->SetInternalFieldCount( + DatabaseSync::kInternalFieldCount); + + SetProtoMethod(isolate, tmpl, "open", DatabaseSync::Open); + SetProtoMethod(isolate, tmpl, "close", DatabaseSync::Close); + SetProtoDispose(isolate, tmpl, DatabaseSync::Dispose); + SetProtoMethod(isolate, tmpl, "prepare", DatabaseSync::Prepare); + SetProtoMethod(isolate, tmpl, "exec", DatabaseSync::Exec); + SetProtoMethod(isolate, tmpl, "function", DatabaseSync::CustomFunction); + SetProtoMethod(isolate, tmpl, "createTagStore", DatabaseSync::CreateTagStore); + SetProtoMethodNoSideEffect(isolate, tmpl, "location", DatabaseSync::Location); + SetProtoMethod(isolate, tmpl, "aggregate", DatabaseSync::AggregateFunction); + SetProtoMethod(isolate, tmpl, "createSession", DatabaseSync::CreateSession); + SetProtoMethod(isolate, tmpl, "applyChangeset", DatabaseSync::ApplyChangeset); + SetProtoMethod( + isolate, tmpl, "enableLoadExtension", DatabaseSync::EnableLoadExtension); + SetProtoMethod( + isolate, tmpl, "enableDefensive", DatabaseSync::EnableDefensive); + SetProtoMethod(isolate, tmpl, "loadExtension", DatabaseSync::LoadExtension); + SetProtoMethod(isolate, tmpl, "setAuthorizer", DatabaseSync::SetAuthorizer); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "isOpen"), + DatabaseSync::IsOpenGetter); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "isTransaction"), + DatabaseSync::IsTransactionGetter); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "limits"), + DatabaseSync::LimitsGetter); + Local sqlite_type_key = FIXED_ONE_BYTE_STRING(isolate, "sqlite-type"); + Local sqlite_type_symbol = + v8::Symbol::For(isolate, sqlite_type_key); + Local database_sync_string = + FIXED_ONE_BYTE_STRING(isolate, "node:sqlite"); + tmpl->InstanceTemplate()->Set(sqlite_type_symbol, database_sync_string); + + return tmpl; +} +} // namespace + bool DatabaseSync::Open() { if (IsOpen()) { THROW_ERR_INVALID_STATE(env(), "database is already open"); @@ -3060,23 +3125,6 @@ SQLTagStore::SQLTagStore(Environment* env, MakeWeak(); } -static inline void SetSideEffectFreeGetter( - Isolate* isolate, - Local class_template, - Local name, - FunctionCallback fn) { - Local getter = - FunctionTemplate::New(isolate, - fn, - Local(), - v8::Signature::New(isolate, class_template), - /* length */ 0, - ConstructorBehavior::kThrow, - SideEffectType::kHasNoSideEffect); - class_template->InstanceTemplate()->SetAccessorProperty( - name, getter, Local(), DontDelete); -} - SQLTagStore::~SQLTagStore() {} Local SQLTagStore::GetConstructorTemplate(Environment* env) { @@ -3722,60 +3770,15 @@ static void Initialize(Local target, void* priv) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); - Local db_tmpl = - NewFunctionTemplate(isolate, DatabaseSync::New); - db_tmpl->InstanceTemplate()->SetInternalFieldCount( - DatabaseSync::kInternalFieldCount); + Local constants = Object::New(isolate); DefineConstants(constants); - SetProtoMethod(isolate, db_tmpl, "open", DatabaseSync::Open); - SetProtoMethod(isolate, db_tmpl, "close", DatabaseSync::Close); - SetProtoDispose(isolate, db_tmpl, DatabaseSync::Dispose); - SetProtoMethod(isolate, db_tmpl, "prepare", DatabaseSync::Prepare); - SetProtoMethod(isolate, db_tmpl, "exec", DatabaseSync::Exec); - SetProtoMethod(isolate, db_tmpl, "function", DatabaseSync::CustomFunction); - SetProtoMethod( - isolate, db_tmpl, "createTagStore", DatabaseSync::CreateTagStore); - SetProtoMethodNoSideEffect( - isolate, db_tmpl, "location", DatabaseSync::Location); - SetProtoMethod( - isolate, db_tmpl, "aggregate", DatabaseSync::AggregateFunction); - SetProtoMethod( - isolate, db_tmpl, "createSession", DatabaseSync::CreateSession); - SetProtoMethod( - isolate, db_tmpl, "applyChangeset", DatabaseSync::ApplyChangeset); - SetProtoMethod(isolate, - db_tmpl, - "enableLoadExtension", - DatabaseSync::EnableLoadExtension); - SetProtoMethod( - isolate, db_tmpl, "enableDefensive", DatabaseSync::EnableDefensive); - SetProtoMethod( - isolate, db_tmpl, "loadExtension", DatabaseSync::LoadExtension); - SetProtoMethod( - isolate, db_tmpl, "setAuthorizer", DatabaseSync::SetAuthorizer); - SetSideEffectFreeGetter(isolate, - db_tmpl, - FIXED_ONE_BYTE_STRING(isolate, "isOpen"), - DatabaseSync::IsOpenGetter); - SetSideEffectFreeGetter(isolate, - db_tmpl, - FIXED_ONE_BYTE_STRING(isolate, "isTransaction"), - DatabaseSync::IsTransactionGetter); - SetSideEffectFreeGetter(isolate, - db_tmpl, - FIXED_ONE_BYTE_STRING(isolate, "limits"), - DatabaseSync::LimitsGetter); - Local sqlite_type_key = FIXED_ONE_BYTE_STRING(isolate, "sqlite-type"); - Local sqlite_type_symbol = - v8::Symbol::For(isolate, sqlite_type_key); - Local database_sync_string = - FIXED_ONE_BYTE_STRING(isolate, "node:sqlite"); - db_tmpl->InstanceTemplate()->Set(sqlite_type_symbol, database_sync_string); - - SetConstructorFunction(context, target, "DatabaseSync", db_tmpl); + SetConstructorFunction(context, + target, + "DatabaseSync", + CreateDatabaseSyncConstructorTemplate(env)); SetConstructorFunction(context, target, "StatementSync", From 5d64e3d93126626d008eb1045d0a54af398fd99b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 19 Feb 2026 11:25:36 +0000 Subject: [PATCH 02/15] sqlite: dedupe `SQLTagStore` prototype instances Previously all `SQLTagStore` instances had unique prototypes. Note that the class and its prototype are currently not exposed on `node:sqlite`, i.e. it currently can't be directly used for `instanceOf` checks. --- src/env_properties.h | 1 + src/node_sqlite.cc | 37 ++++++++++++++++++++----------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index 91e2e06c3c2703..75346b18d2b7df 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -441,6 +441,7 @@ V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \ V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \ V(sqlite_session_constructor_template, v8::FunctionTemplate) \ + V(sqlite_sql_tag_store_constructor_template, v8::FunctionTemplate) \ V(srv_record_template, v8::DictionaryTemplate) \ V(streambaseoutputstream_constructor_template, v8::ObjectTemplate) \ V(tcp_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index db4e37a01aa591..5448ca80584f9a 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3128,24 +3128,27 @@ SQLTagStore::SQLTagStore(Environment* env, SQLTagStore::~SQLTagStore() {} Local SQLTagStore::GetConstructorTemplate(Environment* env) { - Isolate* isolate = env->isolate(); Local tmpl = - NewFunctionTemplate(isolate, IllegalConstructor); - tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "SQLTagStore")); - tmpl->InstanceTemplate()->SetInternalFieldCount( - SQLTagStore::kInternalFieldCount); - SetProtoMethod(isolate, tmpl, "get", Get); - SetProtoMethod(isolate, tmpl, "all", All); - SetProtoMethod(isolate, tmpl, "iterate", Iterate); - SetProtoMethod(isolate, tmpl, "run", Run); - SetProtoMethod(isolate, tmpl, "clear", Clear); - SetSideEffectFreeGetter(isolate, - tmpl, - FIXED_ONE_BYTE_STRING(isolate, "capacity"), - CapacityGetter); - SetSideEffectFreeGetter( - isolate, tmpl, FIXED_ONE_BYTE_STRING(isolate, "db"), DatabaseGetter); - SetSideEffectFreeGetter(isolate, tmpl, env->size_string(), SizeGetter); + env->sqlite_sql_tag_store_constructor_template(); + if (tmpl.IsEmpty()) { + Isolate* isolate = env->isolate(); + tmpl = NewFunctionTemplate(isolate, IllegalConstructor); + tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "SQLTagStore")); + tmpl->InstanceTemplate()->SetInternalFieldCount( + SQLTagStore::kInternalFieldCount); + SetProtoMethod(isolate, tmpl, "get", Get); + SetProtoMethod(isolate, tmpl, "all", All); + SetProtoMethod(isolate, tmpl, "iterate", Iterate); + SetProtoMethod(isolate, tmpl, "run", Run); + SetProtoMethod(isolate, tmpl, "clear", Clear); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "capacity"), + CapacityGetter); + SetSideEffectFreeGetter( + isolate, tmpl, FIXED_ONE_BYTE_STRING(isolate, "db"), DatabaseGetter); + SetSideEffectFreeGetter(isolate, tmpl, env->size_string(), SizeGetter); + } return tmpl; } From 9e288e9db8b71578f0670cb7ccd8a3931e49a686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 19 Feb 2026 14:56:32 +0000 Subject: [PATCH 03/15] sqlite: extract async-agnostic code into a common base The database opening and configuration logic can be shared between the sync and async API variants, therefore extract the shared implementation into a common base class. --- src/node_sqlite.cc | 68 ++++++++++++++++++++++++++++++---------------- src/node_sqlite.h | 50 +++++++++++++++++++++------------- 2 files changed, 75 insertions(+), 43 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 5448ca80584f9a..4b816b5e84a583 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -255,16 +255,19 @@ void JSValueToSQLiteResult(Isolate* isolate, } } +inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, sqlite3* db) { + Local e; + if (CreateSQLiteError(isolate, db).ToLocal(&e)) { + isolate->ThrowException(e); + } +} inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, DatabaseSync* db) { if (db->ShouldIgnoreSQLiteError()) { db->SetIgnoreNextSQLiteError(false); return; } - Local e; - if (CreateSQLiteError(isolate, db->Connection()).ToLocal(&e)) { - isolate->ThrowException(e); - } + THROW_ERR_SQLITE_ERROR(isolate, db->Connection()); } inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) { @@ -299,6 +302,28 @@ inline MaybeLocal NullableSQLiteStringToValue(Isolate* isolate, .As(); } +DatabaseCommon::DatabaseCommon(Environment* env, + Local object, + DatabaseOpenConfiguration&& open_config, + bool allow_load_extension) + : BaseObject(env, object), + open_config_(std::move(open_config)), + allow_load_extension_(allow_load_extension), + enable_load_extension_(allow_load_extension) { + MakeWeak(); +} + +namespace { +void AddDatabaseCommonMethodsToTemplate(Isolate* isolate, + Local tmpl) { + SetProtoMethod(isolate, tmpl, "open", DatabaseCommon::Open); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "isOpen"), + DatabaseCommon::IsOpenGetter); +} +} // namespace + class CustomAggregate { public: explicit CustomAggregate(Environment* env, @@ -875,11 +900,9 @@ DatabaseSync::DatabaseSync(Environment* env, DatabaseOpenConfiguration&& open_config, bool open, bool allow_load_extension) - : BaseObject(env, object), open_config_(std::move(open_config)) { + : DatabaseCommon( + env, object, std::move(open_config), allow_load_extension) { MakeWeak(); - connection_ = nullptr; - allow_load_extension_ = allow_load_extension; - enable_load_extension_ = allow_load_extension; ignore_next_sqlite_error_ = false; if (open) { @@ -931,7 +954,8 @@ v8::Local CreateDatabaseSyncConstructorTemplate( tmpl->InstanceTemplate()->SetInternalFieldCount( DatabaseSync::kInternalFieldCount); - SetProtoMethod(isolate, tmpl, "open", DatabaseSync::Open); + AddDatabaseCommonMethodsToTemplate(isolate, tmpl); + SetProtoMethod(isolate, tmpl, "close", DatabaseSync::Close); SetProtoDispose(isolate, tmpl, DatabaseSync::Dispose); SetProtoMethod(isolate, tmpl, "prepare", DatabaseSync::Prepare); @@ -948,10 +972,6 @@ v8::Local CreateDatabaseSyncConstructorTemplate( isolate, tmpl, "enableDefensive", DatabaseSync::EnableDefensive); SetProtoMethod(isolate, tmpl, "loadExtension", DatabaseSync::LoadExtension); SetProtoMethod(isolate, tmpl, "setAuthorizer", DatabaseSync::SetAuthorizer); - SetSideEffectFreeGetter(isolate, - tmpl, - FIXED_ONE_BYTE_STRING(isolate, "isOpen"), - DatabaseSync::IsOpenGetter); SetSideEffectFreeGetter(isolate, tmpl, FIXED_ONE_BYTE_STRING(isolate, "isTransaction"), @@ -971,7 +991,7 @@ v8::Local CreateDatabaseSyncConstructorTemplate( } } // namespace -bool DatabaseSync::Open() { +bool DatabaseCommon::Open() { if (IsOpen()) { THROW_ERR_INVALID_STATE(env(), "database is already open"); return false; @@ -986,18 +1006,18 @@ bool DatabaseSync::Open() { &connection_, flags | default_flags, nullptr); - CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); r = sqlite3_db_config(connection_, SQLITE_DBCONFIG_DQS_DML, static_cast(open_config_.get_enable_dqs()), nullptr); - CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); r = sqlite3_db_config(connection_, SQLITE_DBCONFIG_DQS_DDL, static_cast(open_config_.get_enable_dqs()), nullptr); - CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); int foreign_keys_enabled; r = sqlite3_db_config( @@ -1005,7 +1025,7 @@ bool DatabaseSync::Open() { SQLITE_DBCONFIG_ENABLE_FKEY, static_cast(open_config_.get_enable_foreign_keys()), &foreign_keys_enabled); - CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); CHECK_EQ(foreign_keys_enabled, open_config_.get_enable_foreign_keys()); int defensive_enabled; @@ -1013,7 +1033,7 @@ bool DatabaseSync::Open() { SQLITE_DBCONFIG_DEFENSIVE, static_cast(open_config_.get_enable_defensive()), &defensive_enabled); - CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); CHECK_EQ(defensive_enabled, open_config_.get_enable_defensive()); sqlite3_busy_timeout(connection_, open_config_.get_timeout()); @@ -1036,7 +1056,7 @@ bool DatabaseSync::Open() { const int load_extension_ret = sqlite3_db_config( connection_, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 1, nullptr); CHECK_ERROR_OR_THROW( - env()->isolate(), this, load_extension_ret, SQLITE_OK, false); + env()->isolate(), connection_, load_extension_ret, SQLITE_OK, false); } return true; @@ -1065,11 +1085,11 @@ void DatabaseSync::UntrackStatement(StatementSync* statement) { } } -inline bool DatabaseSync::IsOpen() { +inline bool DatabaseCommon::IsOpen() { return connection_ != nullptr; } -inline sqlite3* DatabaseSync::Connection() { +inline sqlite3* DatabaseCommon::Connection() { return connection_; } @@ -1410,13 +1430,13 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { env, args.This(), std::move(open_config), open, allow_load_extension); } -void DatabaseSync::Open(const FunctionCallbackInfo& args) { +void DatabaseCommon::Open(const FunctionCallbackInfo& args) { DatabaseSync* db; ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); db->Open(); } -void DatabaseSync::IsOpenGetter(const FunctionCallbackInfo& args) { +void DatabaseCommon::IsOpenGetter(const FunctionCallbackInfo& args) { DatabaseSync* db; ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); args.GetReturnValue().Set(db->IsOpen()); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 9b8fcec2699379..8b3c2a12c56810 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -160,10 +160,39 @@ class StatementExecutionHelper { bool use_big_ints); }; -class DatabaseSync : public BaseObject { +class DatabaseCommon : public BaseObject { + public: + DatabaseCommon(Environment* env, + v8::Local object, + DatabaseOpenConfiguration&& open_config, + bool allow_load_extension); + static void Open(const v8::FunctionCallbackInfo& args); + static void IsOpenGetter(const v8::FunctionCallbackInfo& args); + bool IsOpen(); + bool use_big_ints() const { return open_config_.get_use_big_ints(); } + bool return_arrays() const { return open_config_.get_return_arrays(); } + bool allow_bare_named_params() const { + return open_config_.get_allow_bare_named_params(); + } + bool allow_unknown_named_params() const { + return open_config_.get_allow_unknown_named_params(); + } + sqlite3* Connection(); + + protected: + ~DatabaseCommon() override = default; + bool Open(); + + DatabaseOpenConfiguration open_config_; + sqlite3* connection_ = nullptr; + bool allow_load_extension_; + bool enable_load_extension_; +}; + +class DatabaseSync : public DatabaseCommon { public: enum InternalFields { - kAuthorizerCallback = BaseObject::kInternalFieldCount, + kAuthorizerCallback = DatabaseCommon::kInternalFieldCount, kLimitsObject, kInternalFieldCount }; @@ -175,8 +204,6 @@ class DatabaseSync : public BaseObject { bool allow_load_extension); void MemoryInfo(MemoryTracker* tracker) const override; static void New(const v8::FunctionCallbackInfo& args); - static void Open(const v8::FunctionCallbackInfo& args); - static void IsOpenGetter(const v8::FunctionCallbackInfo& args); static void IsTransactionGetter( const v8::FunctionCallbackInfo& args); static void Close(const v8::FunctionCallbackInfo& args); @@ -207,16 +234,6 @@ class DatabaseSync : public BaseObject { void AddBackup(BackupJob* backup); void FinalizeBackups(); void UntrackStatement(StatementSync* statement); - bool IsOpen(); - bool use_big_ints() const { return open_config_.get_use_big_ints(); } - bool return_arrays() const { return open_config_.get_return_arrays(); } - bool allow_bare_named_params() const { - return open_config_.get_allow_bare_named_params(); - } - bool allow_unknown_named_params() const { - return open_config_.get_allow_unknown_named_params(); - } - sqlite3* Connection(); // In some situations, such as when using custom functions, it is possible // that SQLite reports an error while JavaScript already has a pending @@ -229,14 +246,9 @@ class DatabaseSync : public BaseObject { SET_SELF_SIZE(DatabaseSync) private: - bool Open(); void DeleteSessions(); ~DatabaseSync() override; - DatabaseOpenConfiguration open_config_; - bool allow_load_extension_; - bool enable_load_extension_; - sqlite3* connection_; bool ignore_next_sqlite_error_; std::set backups_; From 2aabaa99ec05a497a7b09fc31ed9d136f89e5020 Mon Sep 17 00:00:00 2001 From: Edy Silva Date: Mon, 1 Dec 2025 12:24:29 -0300 Subject: [PATCH 04/15] sqlite: set threading mode to multithreaded --- deps/sqlite/sqlite.gyp | 1 + deps/sqlite/unofficial.gni | 1 + 2 files changed, 2 insertions(+) diff --git a/deps/sqlite/sqlite.gyp b/deps/sqlite/sqlite.gyp index db57db1f4acdfd..b80d46f1ecc097 100644 --- a/deps/sqlite/sqlite.gyp +++ b/deps/sqlite/sqlite.gyp @@ -26,6 +26,7 @@ 'SQLITE_ENABLE_RBU', 'SQLITE_ENABLE_RTREE', 'SQLITE_ENABLE_SESSION', + 'SQLITE_THREADSAFE=2', ], 'include_dirs': ['.'], 'sources': [ diff --git a/deps/sqlite/unofficial.gni b/deps/sqlite/unofficial.gni index 0e62accfb6990e..f24bf635f65fe4 100644 --- a/deps/sqlite/unofficial.gni +++ b/deps/sqlite/unofficial.gni @@ -20,6 +20,7 @@ template("sqlite_gn_build") { "SQLITE_ENABLE_RBU", "SQLITE_ENABLE_RTREE", "SQLITE_ENABLE_SESSION", + "SQLITE_THREADSAFE=2", ] } From aec981af52efa8e6b571e65f4018110801dbd5c4 Mon Sep 17 00:00:00 2001 From: Edy Silva Date: Thu, 19 Feb 2026 08:27:22 +0000 Subject: [PATCH 05/15] test: add sqlite async test suite --- test/parallel/test-sqlite-database-async.mjs | 657 +++++++++++++++++++ test/parallel/test-sqlite-statement-async.js | 612 +++++++++++++++++ 2 files changed, 1269 insertions(+) create mode 100644 test/parallel/test-sqlite-database-async.mjs create mode 100644 test/parallel/test-sqlite-statement-async.js diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs new file mode 100644 index 00000000000000..3a1322604f5547 --- /dev/null +++ b/test/parallel/test-sqlite-database-async.mjs @@ -0,0 +1,657 @@ +import { skip, skipIfSQLiteMissing } from '../common/index.mjs'; +skip(); +import tmpdir from '../common/tmpdir.js'; +import { existsSync } from 'node:fs'; +import { suite, test } from 'node:test'; +import { join } from 'node:path'; +import { Database, Statement } from 'node:sqlite'; +skipIfSQLiteMissing(); + +tmpdir.refresh(); + +let cnt = 0; +function nextDb() { + return join(tmpdir.path, `database-${cnt++}.db`); +} + +suite('Database() constructor', () => { + test('throws if called without new', (t) => { + t.assert.throws(() => { + Database(); + }, { + code: 'ERR_CONSTRUCT_CALL_REQUIRED', + message: /Cannot call constructor without `new`/, + }); + }); + + test('throws if database path is not a string, Uint8Array, or URL', (t) => { + t.assert.throws(() => { + new Database(); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "path" argument must be a string, Uint8Array, or URL without null bytes/, + }); + }); + + test('throws if the database location as Buffer contains null bytes', (t) => { + t.assert.throws(() => { + new Database(Buffer.from('l\0cation')); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "path" argument must be a string, Uint8Array, or URL without null bytes.', + }); + }); + + test('throws if the database location as string contains null bytes', (t) => { + t.assert.throws(() => { + new Database('l\0cation'); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "path" argument must be a string, Uint8Array, or URL without null bytes.', + }); + }); + + test('throws if options is provided but is not an object', (t) => { + t.assert.throws(() => { + new Database('foo', null); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options" argument must be an object/, + }); + }); + + test('throws if options.open is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { open: 5 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options\.open" argument must be a boolean/, + }); + }); + + test('throws if options.readOnly is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { readOnly: 5 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options\.readOnly" argument must be a boolean/, + }); + }); + + test('throws if options.timeout is provided but is not an integer', (t) => { + t.assert.throws(() => { + new Database('foo', { timeout: .99 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options\.timeout" argument must be an integer/, + }); + }); + + test('is not read-only by default', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath); + await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + }); + + test('is read-only if readOnly is set', async (t) => { + const dbPath = nextDb(); + { + const db = new Database(dbPath); + await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + db.close(); + } + { + const db = new Database(dbPath, { readOnly: true }); + await t.assert.rejects(db.exec('CREATE TABLE bar (id INTEGER PRIMARY KEY)'), { + code: 'ERR_SQLITE_ERROR', + message: /attempt to write a readonly database/, + }); + } + }); + + test('throws if options.enableForeignKeyConstraints is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { enableForeignKeyConstraints: 5 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options\.enableForeignKeyConstraints" argument must be a boolean/, + }); + }); + + test('enables foreign key constraints by default', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath); + await db.exec(` + CREATE TABLE foo (id INTEGER PRIMARY KEY); + CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); + `); + t.after(() => { db.close(); }); + await t.assert.rejects( + db.exec('INSERT INTO bar (foo_id) VALUES (1)'), + { + code: 'ERR_SQLITE_ERROR', + message: 'FOREIGN KEY constraint failed', + }); + }); + + test('allows disabling foreign key constraints', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath, { enableForeignKeyConstraints: false }); + await db.exec(` + CREATE TABLE foo (id INTEGER PRIMARY KEY); + CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); + `); + t.after(() => { db.close(); }); + await db.exec('INSERT INTO bar (foo_id) VALUES (1)'); + }); + + test('throws if options.enableDoubleQuotedStringLiterals is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { enableDoubleQuotedStringLiterals: 5 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options\.enableDoubleQuotedStringLiterals" argument must be a boolean/, + }); + }); + + test('disables double-quoted string literals by default', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath); + t.after(() => { db.close(); }); + await t.assert.rejects(db.exec('SELECT "foo";'), { + code: 'ERR_SQLITE_ERROR', + message: /no such column: "?foo"?/, + }); + }); + + test('allows enabling double-quoted string literals', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath, { enableDoubleQuotedStringLiterals: true }); + t.after(() => { db.close(); }); + await db.exec('SELECT "foo";'); + }); + + test('throws if options.readBigInts is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { readBigInts: 42 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "options.readBigInts" argument must be a boolean.', + }); + }); + + test('allows reading big integers', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath, { readBigInts: true }); + t.after(() => { db.close(); }); + + const setup = await db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT; + INSERT INTO data (key, val) VALUES (1, 42); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT val FROM data'); + t.assert.deepStrictEqual(await query.get(), { __proto__: null, val: 42n }); + + const insert = db.prepare('INSERT INTO data (key) VALUES (?)'); + t.assert.deepStrictEqual( + await insert.run(20), + { changes: 1n, lastInsertRowid: 20n }, + ); + }); + + test('throws if options.returnArrays is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { returnArrays: 42 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "options.returnArrays" argument must be a boolean.', + }); + }); + + test('allows returning arrays', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath, { returnArrays: true }); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY, val TEXT) STRICT; + INSERT INTO data (key, val) VALUES (1, 'one'); + INSERT INTO data (key, val) VALUES (2, 'two'); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT key, val FROM data WHERE key = 1'); + t.assert.deepStrictEqual(await query.get(), [1, 'one']); + }); + + test('throws if options.allowBareNamedParameters is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { allowBareNamedParameters: 42 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "options.allowBareNamedParameters" argument must be a boolean.', + }); + }); + + test('throws if bare named parameters are used when option is false', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath, { allowBareNamedParameters: false }); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + + const stmt = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); + t.assert.throws(() => { + stmt.run({ k: 2, v: 4 }); + }, { + code: 'ERR_INVALID_STATE', + message: /Unknown named parameter 'k'/, + }); + }); + + test('throws if options.allowUnknownNamedParameters is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new Database('foo', { allowUnknownNamedParameters: 42 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "options.allowUnknownNamedParameters" argument must be a boolean.', + }); + }); + + test('allows unknown named parameters', async (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath, { allowUnknownNamedParameters: true }); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + + const stmt = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); + const params = { $a: 1, $b: 2, $k: 42, $y: 25, $v: 84, $z: 99 }; + t.assert.deepStrictEqual( + await stmt.run(params), + { changes: 1, lastInsertRowid: 1 }, + ); + }); + + test('has sqlite-type symbol property', (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath); + t.after(() => { db.close(); }); + + const sqliteTypeSymbol = Symbol.for('sqlite-type'); + t.assert.strictEqual(db[sqliteTypeSymbol], 'node:sqlite'); + }); +}); + +suite('Database.prototype.open()', () => { + test('opens a database connection', (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath, { open: false }); + t.after(() => { db.close(); }); + + t.assert.strictEqual(db.isOpen, false); + t.assert.strictEqual(existsSync(dbPath), false); + t.assert.strictEqual(db.open(), undefined); + t.assert.strictEqual(db.isOpen, true); + t.assert.strictEqual(existsSync(dbPath), true); + }); + + test('throws if database is already open', (t) => { + const db = new Database(nextDb(), { open: false }); + t.after(() => { db.close(); }); + + t.assert.strictEqual(db.isOpen, false); + db.open(); + t.assert.strictEqual(db.isOpen, true); + t.assert.throws(() => { + db.open(); + }, { + code: 'ERR_INVALID_STATE', + message: /database is already open/, + }); + t.assert.strictEqual(db.isOpen, true); + }); +}); + +suite('Database.prototype.close()', () => { + test('closes an open database connection', (t) => { + const db = new Database(nextDb()); + + t.assert.strictEqual(db.isOpen, true); + t.assert.strictEqual(db.close(), undefined); + t.assert.strictEqual(db.isOpen, false); + }); + + test('throws if database is not open', (t) => { + const db = new Database(nextDb(), { open: false }); + + t.assert.strictEqual(db.isOpen, false); + t.assert.throws(() => { + db.close(); + }, { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + t.assert.strictEqual(db.isOpen, false); + }); +}); + +suite('Database.prototype.prepare()', () => { + test('returns a prepared statement', (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const stmt = db.prepare('CREATE TABLE webstorage(key TEXT)'); + t.assert.ok(stmt instanceof Statement); + }); + + test('throws if database is not open', (t) => { + const db = new Database(nextDb(), { open: false }); + + t.assert.throws(() => { + db.prepare(); + }, { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + }); + + test('throws if sql is not a string', (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + + t.assert.throws(() => { + db.prepare(); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "sql" argument must be a string/, + }); + }); +}); + +suite('Database.prototype.exec()', () => { + test('executes SQL', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const result = await db.exec(` + CREATE TABLE data( + key INTEGER PRIMARY KEY, + val INTEGER + ) STRICT; + INSERT INTO data (key, val) VALUES (1, 2); + INSERT INTO data (key, val) VALUES (8, 9); + `); + t.assert.strictEqual(result, undefined); + const stmt = db.prepare('SELECT * FROM data ORDER BY key'); + t.assert.deepStrictEqual(await stmt.all(), [ + { __proto__: null, key: 1, val: 2 }, + { __proto__: null, key: 8, val: 9 }, + ]); + }); + + test('reports errors from SQLite', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + + await t.assert.rejects( + db.exec('CREATE TABLEEEE'), + { + code: 'ERR_SQLITE_ERROR', + message: /syntax error/, + }); + }); + + test('throws if the URL does not have the file: scheme', (t) => { + t.assert.throws(() => { + new Database(new URL('http://example.com')); + }, { + code: 'ERR_INVALID_URL_SCHEME', + message: 'The URL must be of scheme file:', + }); + }); + + test('throws if database is not open', (t) => { + const db = new Database(nextDb(), { open: false }); + + t.assert.throws(() => { + db.exec(); + }, { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + }); + + test('throws if sql is not a string', (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + + t.assert.throws(() => { + db.exec(); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "sql" argument must be a string/, + }); + }); +}); + +suite('Database.prototype.isTransaction', () => { + test('correctly detects a committed transaction', async (t) => { + const db = new Database(':memory:'); + + t.assert.strictEqual(db.isTransaction, false); + await db.exec('BEGIN'); + t.assert.strictEqual(db.isTransaction, true); + await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + t.assert.strictEqual(db.isTransaction, true); + await db.exec('COMMIT'); + t.assert.strictEqual(db.isTransaction, false); + }); + + test('correctly detects a rolled back transaction', async (t) => { + const db = new Database(':memory:'); + + t.assert.strictEqual(db.isTransaction, false); + await db.exec('BEGIN'); + t.assert.strictEqual(db.isTransaction, true); + await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + t.assert.strictEqual(db.isTransaction, true); + await db.exec('ROLLBACK'); + t.assert.strictEqual(db.isTransaction, false); + }); + + test('throws if database is not open', (t) => { + const db = new Database(nextDb(), { open: false }); + + t.assert.throws(() => { + return db.isTransaction; + }, { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + }); +}); + +suite('Database.prototype.location()', () => { + test('throws if database is not open', (t) => { + const db = new Database(nextDb(), { open: false }); + + t.assert.throws(() => { + db.location(); + }, { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + }); + + test('throws if provided dbName is not string', (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + + t.assert.throws(() => { + db.location(null); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "dbName" argument must be a string/, + }); + }); + + test('returns null when connected to in-memory database', (t) => { + const db = new Database(':memory:'); + t.assert.strictEqual(db.location(), null); + }); + + test('returns db path when connected to a persistent database', (t) => { + const dbPath = nextDb(); + const db = new Database(dbPath); + t.after(() => { db.close(); }); + t.assert.strictEqual(db.location(), dbPath); + }); + + test('returns that specific db path when attached', async (t) => { + const dbPath = nextDb(); + const otherPath = nextDb(); + const db = new Database(dbPath); + t.after(() => { db.close(); }); + const other = new Database(dbPath); + t.after(() => { other.close(); }); + + // Adding this escape because the test with unusual chars have a single quote which breaks the query + const escapedPath = otherPath.replace("'", "''"); + await db.exec(`ATTACH DATABASE '${escapedPath}' AS other`); + + t.assert.strictEqual(db.location('other'), otherPath); + }); +}); + +suite('Async mode restrictions', () => { + test('throws when defining a custom function', (t) => { + const db = new Database(':memory:'); + t.after(() => { db.close(); }); + + t.assert.throws(() => { + db.function('test', () => 1); + }, { + code: 'ERR_INVALID_STATE', + message: /Custom functions are not supported in async mode/, + }); + }); + + test('throws when defining an aggregate function', (t) => { + const db = new Database(':memory:'); + t.after(() => { db.close(); }); + + t.assert.throws(() => { + db.aggregate('test', { start: 0, step: (acc) => acc }); + }, { + code: 'ERR_INVALID_STATE', + message: /Aggregate functions are not supported in async mode/, + }); + }); + + test('throws when setting an authorizer callback', (t) => { + const db = new Database(':memory:'); + t.after(() => { db.close(); }); + + t.assert.throws(() => { + db.setAuthorizer(() => 0); + }, { + code: 'ERR_INVALID_STATE', + message: /Authorizer callbacks are not supported in async mode/, + }); + }); +}); + +suite('Async operation ordering', () => { + test('executes operations sequentially per database', async (t) => { + const db = new Database(':memory:'); + t.after(() => { db.close(); }); + await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, seq INTEGER)'); + + // Launch multiple operations concurrently + const ops = []; + for (let i = 0; i < 10; i++) { + ops.push(db.exec(`INSERT INTO test (seq) VALUES (${i})`)); + } + + await Promise.all(ops); + + // Check they were inserted in order (sequential execution) + const stmt = db.prepare('SELECT id, seq FROM test ORDER BY id'); + const rows = await stmt.all(); + + // Verify sequential: id should match seq + 1 (autoincrement starts at 1) + t.assert.strictEqual(rows.length, 10); + for (const row of rows) { + t.assert.strictEqual(row.id, row.seq + 1); + } + }); + + test('different connections can execute in parallel', async (t) => { + const db1 = new Database(':memory:'); + const db2 = new Database(':memory:'); + t.after(() => { db1.close(); db2.close(); }); + const times = {}; + const now = () => process.hrtime.bigint(); + const LONG_QUERY = ` + WITH RECURSIVE cnt(x) AS ( + SELECT 1 + UNION ALL + SELECT x + 1 FROM cnt WHERE x < 300000 + ) + SELECT sum(x) FROM cnt; + `; + + const op = async (db, label) => { + times[label] = { start: now() }; + + await db.exec(LONG_QUERY); + + times[label].end = now(); + }; + + // Start both operations + await Promise.all([op(db1, 'db1'), op(db2, 'db2')]); + + // Verify that their execution times overlap + t.assert.ok( + times.db1.start < times.db2.end && + times.db2.start < times.db1.end + ); + }); +}); + +suite('Database.prototype.close', () => { + test('rejects pending operations when database is closed', async (t) => { + const db = new Database(':memory:'); + await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)'); + const pendingOp = db.exec('INSERT INTO test (value) VALUES (\'test\')'); + db.close(); + + await t.assert.rejects(pendingOp, { + code: 'ERR_INVALID_STATE', + message: /database is closing/, + }); + }); + + test('rejects multiple pending operations when database is closed', async (t) => { + const db = new Database(':memory:'); + await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)'); + const ops = [ + db.exec('INSERT INTO test (value) VALUES (\'test1\')'), + db.exec('INSERT INTO test (value) VALUES (\'test2\')'), + db.exec('INSERT INTO test (value) VALUES (\'test3\')'), + ]; + + db.close(); + + const results = await Promise.allSettled(ops); + for (const result of results) { + t.assert.strictEqual(result.status, 'rejected'); + t.assert.strictEqual(result.reason.code, 'ERR_INVALID_STATE'); + } + }); +}); diff --git a/test/parallel/test-sqlite-statement-async.js b/test/parallel/test-sqlite-statement-async.js new file mode 100644 index 00000000000000..517c07cbf60a3a --- /dev/null +++ b/test/parallel/test-sqlite-statement-async.js @@ -0,0 +1,612 @@ +// Flags: --expose-gc +'use strict'; +const { skip, skipIfSQLiteMissing } = require('../common'); +skip(); // TODO: Re-enable when async Statement support is added. +skipIfSQLiteMissing(); +const tmpdir = require('../common/tmpdir'); +const { join } = require('node:path'); +const { Database, Statement } = require('node:sqlite'); +const { suite, test } = require('node:test'); +let cnt = 0; + +tmpdir.refresh(); + +function nextDb() { + return join(tmpdir.path, `database-${cnt++}.db`); +} + +suite('Statement() constructor', () => { + test('Statement cannot be constructed directly', (t) => { + t.assert.throws(() => { + new Statement(); + }, { + code: 'ERR_ILLEGAL_CONSTRUCTOR', + message: /Illegal constructor/, + }); + }); +}); + +suite('Statement.prototype.get()', () => { + test('executes a query and returns undefined on no results', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.strictEqual(await stmt.get(), undefined); + stmt = db.prepare('SELECT * FROM storage'); + t.assert.strictEqual(await stmt.get(), undefined); + }); + + test('executes a query and returns the first result', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.strictEqual(await stmt.get(), undefined); + stmt = db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)'); + t.assert.strictEqual(await stmt.get('key1', 'val1'), undefined); + t.assert.strictEqual(await stmt.get('key2', 'val2'), undefined); + stmt = db.prepare('SELECT * FROM storage ORDER BY key'); + t.assert.deepStrictEqual(await stmt.get(), { __proto__: null, key: 'key1', val: 'val1' }); + }); + + test('executes a query that returns special columns', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const stmt = db.prepare('SELECT 1 as __proto__, 2 as constructor, 3 as toString'); + t.assert.deepStrictEqual(await stmt.get(), { __proto__: null, ['__proto__']: 1, constructor: 2, toString: 3 }); + }); +}); + +suite('Statement.prototype.all()', () => { + test('executes a query and returns an empty array on no results', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.deepStrictEqual(await stmt.all(), []); + }); + + test('executes a query and returns all results', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.deepStrictEqual(await stmt.run(), { changes: 0, lastInsertRowid: 0 }); + stmt = db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)'); + t.assert.deepStrictEqual( + await stmt.run('key1', 'val1'), + { changes: 1, lastInsertRowid: 1 }, + ); + t.assert.deepStrictEqual( + await stmt.run('key2', 'val2'), + { changes: 1, lastInsertRowid: 2 }, + ); + stmt = db.prepare('SELECT * FROM storage ORDER BY key'); + t.assert.deepStrictEqual(await stmt.all(), [ + { __proto__: null, key: 'key1', val: 'val1' }, + { __proto__: null, key: 'key2', val: 'val2' }, + ]); + }); +}); + +suite('Statement.prototype.iterate()', () => { + test('executes a query and returns an empty iterator on no results', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + const iter = stmt.iterate(); + t.assert.strictEqual(iter instanceof globalThis.Iterator, true); + t.assert.ok(iter[Symbol.iterator]); + t.assert.deepStrictEqual(iter.toArray(), []); + }); + + test('executes a query and returns all results', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.deepStrictEqual(await stmt.run(), { changes: 0, lastInsertRowid: 0 }); + stmt = db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)'); + t.assert.deepStrictEqual( + await stmt.run('key1', 'val1'), + { changes: 1, lastInsertRowid: 1 }, + ); + t.assert.deepStrictEqual( + await stmt.run('key2', 'val2'), + { changes: 1, lastInsertRowid: 2 }, + ); + + const items = [ + { __proto__: null, key: 'key1', val: 'val1' }, + { __proto__: null, key: 'key2', val: 'val2' }, + ]; + + stmt = db.prepare('SELECT * FROM storage ORDER BY key'); + t.assert.deepStrictEqual(stmt.iterate().toArray(), items); + + const itemsLoop = items.slice(); + for (const item of stmt.iterate()) { + t.assert.deepStrictEqual(item, itemsLoop.shift()); + } + }); + + test('iterator keeps the prepared statement from being collected', async (t) => { + const db = new Database(':memory:'); + await db.exec(` + CREATE TABLE test(key TEXT, val TEXT); + INSERT INTO test (key, val) VALUES ('key1', 'val1'); + INSERT INTO test (key, val) VALUES ('key2', 'val2'); + `); + // Do not keep an explicit reference to the prepared statement. + const iterator = db.prepare('SELECT * FROM test').iterate(); + const results = []; + + global.gc(); + + for (const item of iterator) { + results.push(item); + } + + t.assert.deepStrictEqual(results, [ + { __proto__: null, key: 'key1', val: 'val1' }, + { __proto__: null, key: 'key2', val: 'val2' }, + ]); + }); + + test('iterator can be exited early', async (t) => { + const db = new Database(':memory:'); + await db.exec(` + CREATE TABLE test(key TEXT, val TEXT); + INSERT INTO test (key, val) VALUES ('key1', 'val1'); + INSERT INTO test (key, val) VALUES ('key2', 'val2'); + `); + const iterator = db.prepare('SELECT * FROM test').iterate(); + const results = []; + + for (const item of iterator) { + results.push(item); + break; + } + + t.assert.deepStrictEqual(results, [ + { __proto__: null, key: 'key1', val: 'val1' }, + ]); + t.assert.deepStrictEqual( + iterator.next(), + { __proto__: null, done: true, value: null }, + ); + }); +}); + +suite('Statement.prototype.run()', () => { + test('executes a query and returns change metadata', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE storage(key TEXT, val TEXT); + INSERT INTO storage (key, val) VALUES ('foo', 'bar'); + `); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('SELECT * FROM storage'); + t.assert.deepStrictEqual(await stmt.run(), { changes: 1, lastInsertRowid: 1 }); + }); + + test('SQLite throws when trying to bind too many parameters', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?, ?)'); + t.assert.throws(() => { + stmt.run(1, 2, 3); + }, { + code: 'ERR_SQLITE_ERROR', + message: 'column index out of range', + errcode: 25, + errstr: 'column index out of range', + }); + }); + + test('SQLite defaults to NULL for unbound parameters', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?, ?)'); + await t.assert.rejects( + stmt.run(1), + { + code: 'ERR_SQLITE_ERROR', + message: 'NOT NULL constraint failed: data.val', + errcode: 1299, + errstr: 'constraint failed', + }); + }); + + test('returns correct metadata when using RETURNING', async (t) => { + const db = new Database(':memory:'); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const sql = 'INSERT INTO data (key, val) VALUES ($k, $v) RETURNING key'; + const stmt = db.prepare(sql); + t.assert.deepStrictEqual( + await stmt.run({ k: 1, v: 10 }), { changes: 1, lastInsertRowid: 1 } + ); + t.assert.deepStrictEqual( + await stmt.run({ k: 2, v: 20 }), { changes: 1, lastInsertRowid: 2 } + ); + t.assert.deepStrictEqual( + await stmt.run({ k: 3, v: 30 }), { changes: 1, lastInsertRowid: 3 } + ); + }); + + test('SQLite defaults unbound ?NNN parameters', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?1, ?3)'); + + await t.assert.rejects( + stmt.run(1), + { + code: 'ERR_SQLITE_ERROR', + message: 'NOT NULL constraint failed: data.val', + errcode: 1299, + errstr: 'constraint failed', + }); + }); + + test('binds ?NNN params by position', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?1, ?2)'); + t.assert.deepStrictEqual(await stmt.run(1, 2), { changes: 1, lastInsertRowid: 1 }); + }); +}); + +suite('Statement.prototype.sourceSQL', () => { + test('equals input SQL', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE types(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const sql = 'INSERT INTO types (key, val) VALUES ($k, $v)'; + const stmt = db.prepare(sql); + t.assert.strictEqual(stmt.sourceSQL, sql); + }); +}); + +suite('Statement.prototype.expandedSQL', async () => { + test('equals expanded SQL', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE types(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const sql = 'INSERT INTO types (key, val) VALUES ($k, ?)'; + const expanded = 'INSERT INTO types (key, val) VALUES (\'33\', \'42\')'; + const stmt = db.prepare(sql); + t.assert.deepStrictEqual( + await stmt.run({ $k: '33' }, '42'), + { changes: 1, lastInsertRowid: 33 }, + ); + t.assert.strictEqual(stmt.expandedSQL, expanded); + }); +}); + +suite('Statement.prototype.setReadBigInts()', () => { + test('BigInts support can be toggled', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT; + INSERT INTO data (key, val) VALUES (1, 42); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT val FROM data'); + t.assert.deepStrictEqual(await query.get(), { __proto__: null, val: 42 }); + t.assert.strictEqual(query.setReadBigInts(true), undefined); + t.assert.deepStrictEqual(await query.get(), { __proto__: null, val: 42n }); + t.assert.strictEqual(query.setReadBigInts(false), undefined); + t.assert.deepStrictEqual(await query.get(), { __proto__: null, val: 42 }); + + const insert = db.prepare('INSERT INTO data (key) VALUES (?)'); + t.assert.deepStrictEqual( + await insert.run(10), + { changes: 1, lastInsertRowid: 10 }, + ); + t.assert.strictEqual(insert.setReadBigInts(true), undefined); + t.assert.deepStrictEqual( + await insert.run(20), + { changes: 1n, lastInsertRowid: 20n }, + ); + t.assert.strictEqual(insert.setReadBigInts(false), undefined); + t.assert.deepStrictEqual( + await insert.run(30), + { changes: 1, lastInsertRowid: 30 }, + ); + }); + + test('throws when input is not a boolean', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE types(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('INSERT INTO types (key, val) VALUES ($k, $v)'); + t.assert.throws(() => { + stmt.setReadBigInts(); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "readBigInts" argument must be a boolean/, + }); + }); + + test('BigInt is required for reading large integers', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const bad = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`); + await t.assert.rejects( + bad.get(), + { + code: 'ERR_OUT_OF_RANGE', + message: /^Value is too large to be represented as a JavaScript number: 9007199254740992$/, + }); + const good = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`); + good.setReadBigInts(true); + t.assert.deepStrictEqual(await good.get(), { + __proto__: null, + [`${Number.MAX_SAFE_INTEGER} + 1`]: 2n ** 53n, + }); + }); +}); + +suite('Statement.prototype.setReturnArrays()', () => { + test('throws when input is not a boolean', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('SELECT key, val FROM data'); + t.assert.throws(() => { + stmt.setReturnArrays(); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "returnArrays" argument must be a boolean/, + }); + }); +}); + +suite('Statement.prototype.get() with array output', () => { + test('returns array row when setReturnArrays is true', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY, val TEXT) STRICT; + INSERT INTO data (key, val) VALUES (1, 'one'); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT key, val FROM data WHERE key = 1'); + t.assert.deepStrictEqual(await query.get(), { __proto__: null, key: 1, val: 'one' }); + + query.setReturnArrays(true); + t.assert.deepStrictEqual(await query.get(), [1, 'one']); + + query.setReturnArrays(false); + t.assert.deepStrictEqual(await query.get(), { __proto__: null, key: 1, val: 'one' }); + }); + + test('returns array rows with BigInts when both flags are set', async (t) => { + const expected = [1n, 9007199254740992n]; + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE big_data(id INTEGER, big_num INTEGER); + INSERT INTO big_data VALUES (1, 9007199254740992); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT id, big_num FROM big_data'); + query.setReturnArrays(true); + query.setReadBigInts(true); + + const row = await query.get(); + t.assert.deepStrictEqual(row, expected); + }); +}); + +suite('Statement.prototype.all() with array output', () => { + test('returns array rows when setReturnArrays is true', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY, val TEXT) STRICT; + INSERT INTO data (key, val) VALUES (1, 'one'); + INSERT INTO data (key, val) VALUES (2, 'two'); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT key, val FROM data ORDER BY key'); + t.assert.deepStrictEqual(await query.all(), [ + { __proto__: null, key: 1, val: 'one' }, + { __proto__: null, key: 2, val: 'two' }, + ]); + + query.setReturnArrays(true); + t.assert.deepStrictEqual(await query.all(), [ + [1, 'one'], + [2, 'two'], + ]); + + query.setReturnArrays(false); + t.assert.deepStrictEqual(await query.all(), [ + { __proto__: null, key: 1, val: 'one' }, + { __proto__: null, key: 2, val: 'two' }, + ]); + }); + + test('handles array rows with many columns', async (t) => { + const expected = [ + 1, + 'text1', + 1.1, + new Uint8Array([0xde, 0xad, 0xbe, 0xef]), + 5, + 'text2', + 2.2, + new Uint8Array([0xbe, 0xef, 0xca, 0xfe]), + 9, + 'text3', + ]; + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE wide_table( + col1 INTEGER, col2 TEXT, col3 REAL, col4 BLOB, col5 INTEGER, + col6 TEXT, col7 REAL, col8 BLOB, col9 INTEGER, col10 TEXT + ); + INSERT INTO wide_table VALUES ( + 1, 'text1', 1.1, X'DEADBEEF', 5, + 'text2', 2.2, X'BEEFCAFE', 9, 'text3' + ); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT * FROM wide_table'); + query.setReturnArrays(true); + + const results = await query.all(); + t.assert.strictEqual(results.length, 1); + t.assert.deepStrictEqual(results[0], expected); + }); +}); + +suite('Statement.prototype.iterate() with array output', () => { + test('iterates array rows when setReturnArrays is true', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY, val TEXT) STRICT; + INSERT INTO data (key, val) VALUES (1, 'one'); + INSERT INTO data (key, val) VALUES (2, 'two'); + `); + t.assert.strictEqual(setup, undefined); + + const query = db.prepare('SELECT key, val FROM data ORDER BY key'); + + // Test with objects first + const objectRows = []; + for (const row of query.iterate()) { + objectRows.push(row); + } + t.assert.deepStrictEqual(objectRows, [ + { __proto__: null, key: 1, val: 'one' }, + { __proto__: null, key: 2, val: 'two' }, + ]); + + // Test with arrays + query.setReturnArrays(true); + const arrayRows = []; + for (const row of query.iterate()) { + arrayRows.push(row); + } + t.assert.deepStrictEqual(arrayRows, [ + [1, 'one'], + [2, 'two'], + ]); + + // Test toArray() method + t.assert.deepStrictEqual(query.iterate().toArray(), [ + [1, 'one'], + [2, 'two'], + ]); + }); + + test('iterator can be exited early with array rows', async (t) => { + const db = new Database(':memory:'); + await db.exec(` + CREATE TABLE test(key TEXT, val TEXT); + INSERT INTO test (key, val) VALUES ('key1', 'val1'); + INSERT INTO test (key, val) VALUES ('key2', 'val2'); + `); + const stmt = db.prepare('SELECT key, val FROM test'); + stmt.setReturnArrays(true); + + const iterator = stmt.iterate(); + const results = []; + + for (const item of iterator) { + results.push(item); + break; + } + + t.assert.deepStrictEqual(results, [ + ['key1', 'val1'], + ]); + t.assert.deepStrictEqual( + iterator.next(), + { __proto__: null, done: true, value: null }, + ); + }); +}); + +suite('Statement.prototype.setAllowBareNamedParameters()', () => { + test('bare named parameter support can be toggled', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); + t.assert.deepStrictEqual( + await stmt.run({ k: 1, v: 2 }), + { changes: 1, lastInsertRowid: 1 }, + ); + t.assert.strictEqual(stmt.setAllowBareNamedParameters(false), undefined); + t.assert.throws(() => { + stmt.run({ k: 2, v: 4 }); + }, { + code: 'ERR_INVALID_STATE', + message: /Unknown named parameter 'k'/, + }); + t.assert.strictEqual(stmt.setAllowBareNamedParameters(true), undefined); + t.assert.deepStrictEqual( + await stmt.run({ k: 3, v: 6 }), + { changes: 1, lastInsertRowid: 3 }, + ); + }); + + test('throws when input is not a boolean', async (t) => { + const db = new Database(nextDb()); + t.after(() => { db.close(); }); + const setup = await db.exec( + 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' + ); + t.assert.strictEqual(setup, undefined); + const stmt = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); + t.assert.throws(() => { + stmt.setAllowBareNamedParameters(); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "allowBareNamedParameters" argument must be a boolean/, + }); + }); +}); From 654ebd5db41f6f43bf6ab32b356b589a75f202bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 19 Feb 2026 15:08:04 +0000 Subject: [PATCH 06/15] sqlite: extract common database ctor option parsing --- src/node_sqlite.cc | 422 +++++++++++++++++++++++---------------------- 1 file changed, 216 insertions(+), 206 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 4b816b5e84a583..ed253e2535b5b6 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1124,6 +1124,7 @@ void DatabaseSync::CreateTagStore(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(session->object()); } +namespace { std::optional ValidateDatabasePath(Environment* env, Local path, const std::string& field_name) { @@ -1171,258 +1172,267 @@ std::optional ValidateDatabasePath(Environment* env, return std::nullopt; } -void DatabaseSync::New(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - if (!args.IsConstructCall()) { - THROW_ERR_CONSTRUCT_CALL_REQUIRED(env); - return; +bool ParseCommonDatabaseOptions(Environment* env, + Local options, + DatabaseOpenConfiguration& open_config, + bool& open, + bool& allow_load_extension) { + Local open_string = FIXED_ONE_BYTE_STRING(env->isolate(), "open"); + Local open_v; + if (!options->Get(env->context(), open_string).ToLocal(&open_v)) { + return false; } - - std::optional location = - ValidateDatabasePath(env, args[0], "path"); - if (!location.has_value()) { - return; + if (!open_v->IsUndefined()) { + if (!open_v->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), "The \"options.open\" argument must be a boolean."); + return false; + } + open = open_v.As()->Value(); } - DatabaseOpenConfiguration open_config(std::move(location.value())); - bool open = true; - bool allow_load_extension = false; - if (args.Length() > 1) { - if (!args[1]->IsObject()) { - THROW_ERR_INVALID_ARG_TYPE(env->isolate(), - "The \"options\" argument must be an object."); - return; + Local read_only_string = + FIXED_ONE_BYTE_STRING(env->isolate(), "readOnly"); + Local read_only_v; + if (!options->Get(env->context(), read_only_string).ToLocal(&read_only_v)) { + return false; + } + if (!read_only_v->IsUndefined()) { + if (!read_only_v->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"options.readOnly\" argument must be a boolean."); + return false; } + open_config.set_read_only(read_only_v.As()->Value()); + } - Local options = args[1].As(); - Local open_string = FIXED_ONE_BYTE_STRING(env->isolate(), "open"); - Local open_v; - if (!options->Get(env->context(), open_string).ToLocal(&open_v)) { - return; - } - if (!open_v->IsUndefined()) { - if (!open_v->IsBoolean()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), "The \"options.open\" argument must be a boolean."); - return; - } - open = open_v.As()->Value(); + Local enable_foreign_keys_string = + FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints"); + Local enable_foreign_keys_v; + if (!options->Get(env->context(), enable_foreign_keys_string) + .ToLocal(&enable_foreign_keys_v)) { + return false; + } + if (!enable_foreign_keys_v->IsUndefined()) { + if (!enable_foreign_keys_v->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"options.enableForeignKeyConstraints\" argument must be a " + "boolean."); + return false; } + open_config.set_enable_foreign_keys( + enable_foreign_keys_v.As()->Value()); + } - Local read_only_string = - FIXED_ONE_BYTE_STRING(env->isolate(), "readOnly"); - Local read_only_v; - if (!options->Get(env->context(), read_only_string).ToLocal(&read_only_v)) { - return; + Local enable_dqs_string = + FIXED_ONE_BYTE_STRING(env->isolate(), "enableDoubleQuotedStringLiterals"); + Local enable_dqs_v; + if (!options->Get(env->context(), enable_dqs_string).ToLocal(&enable_dqs_v)) { + return false; + } + if (!enable_dqs_v->IsUndefined()) { + if (!enable_dqs_v->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"options.enableDoubleQuotedStringLiterals\" argument must be " + "a boolean."); + return false; } - if (!read_only_v->IsUndefined()) { - if (!read_only_v->IsBoolean()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"options.readOnly\" argument must be a boolean."); - return; - } - open_config.set_read_only(read_only_v.As()->Value()); + open_config.set_enable_dqs(enable_dqs_v.As()->Value()); + } + + Local allow_extension_string = + FIXED_ONE_BYTE_STRING(env->isolate(), "allowExtension"); + Local allow_extension_v; + if (!options->Get(env->context(), allow_extension_string) + .ToLocal(&allow_extension_v)) { + return false; + } + + if (!allow_extension_v->IsUndefined()) { + if (!allow_extension_v->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"options.allowExtension\" argument must be a boolean."); + return false; } + allow_load_extension = allow_extension_v.As()->Value(); + } - Local enable_foreign_keys_string = - FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints"); - Local enable_foreign_keys_v; - if (!options->Get(env->context(), enable_foreign_keys_string) - .ToLocal(&enable_foreign_keys_v)) { - return; + Local timeout_v; + if (!options->Get(env->context(), env->timeout_string()) + .ToLocal(&timeout_v)) { + return false; + } + + if (!timeout_v->IsUndefined()) { + if (!timeout_v->IsInt32()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"options.timeout\" argument must be an integer."); + return false; } - if (!enable_foreign_keys_v->IsUndefined()) { - if (!enable_foreign_keys_v->IsBoolean()) { + + open_config.set_timeout(timeout_v.As()->Value()); + } + + Local read_bigints_v; + if (options->Get(env->context(), env->read_bigints_string()) + .ToLocal(&read_bigints_v)) { + if (!read_bigints_v->IsUndefined()) { + if (!read_bigints_v->IsBoolean()) { THROW_ERR_INVALID_ARG_TYPE( env->isolate(), - "The \"options.enableForeignKeyConstraints\" argument must be a " - "boolean."); - return; + R"(The "options.readBigInts" argument must be a boolean.)"); + return false; } - open_config.set_enable_foreign_keys( - enable_foreign_keys_v.As()->Value()); + open_config.set_use_big_ints(read_bigints_v.As()->Value()); } + } - Local enable_dqs_string = FIXED_ONE_BYTE_STRING( - env->isolate(), "enableDoubleQuotedStringLiterals"); - Local enable_dqs_v; - if (!options->Get(env->context(), enable_dqs_string) - .ToLocal(&enable_dqs_v)) { - return; - } - if (!enable_dqs_v->IsUndefined()) { - if (!enable_dqs_v->IsBoolean()) { + Local return_arrays_v; + if (options->Get(env->context(), env->return_arrays_string()) + .ToLocal(&return_arrays_v)) { + if (!return_arrays_v->IsUndefined()) { + if (!return_arrays_v->IsBoolean()) { THROW_ERR_INVALID_ARG_TYPE( env->isolate(), - "The \"options.enableDoubleQuotedStringLiterals\" argument must be " - "a boolean."); - return; + R"(The "options.returnArrays" argument must be a boolean.)"); + return false; } - open_config.set_enable_dqs(enable_dqs_v.As()->Value()); + open_config.set_return_arrays(return_arrays_v.As()->Value()); } + } - Local allow_extension_string = - FIXED_ONE_BYTE_STRING(env->isolate(), "allowExtension"); - Local allow_extension_v; - if (!options->Get(env->context(), allow_extension_string) - .ToLocal(&allow_extension_v)) { - return; + Local allow_bare_named_params_v; + if (options->Get(env->context(), env->allow_bare_named_params_string()) + .ToLocal(&allow_bare_named_params_v)) { + if (!allow_bare_named_params_v->IsUndefined()) { + if (!allow_bare_named_params_v->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + R"(The "options.allowBareNamedParameters" )" + "argument must be a boolean."); + return false; + } + open_config.set_allow_bare_named_params( + allow_bare_named_params_v.As()->Value()); } + } - if (!allow_extension_v->IsUndefined()) { - if (!allow_extension_v->IsBoolean()) { + Local allow_unknown_named_params_v; + if (options->Get(env->context(), env->allow_unknown_named_params_string()) + .ToLocal(&allow_unknown_named_params_v)) { + if (!allow_unknown_named_params_v->IsUndefined()) { + if (!allow_unknown_named_params_v->IsBoolean()) { THROW_ERR_INVALID_ARG_TYPE( env->isolate(), - "The \"options.allowExtension\" argument must be a boolean."); - return; + R"(The "options.allowUnknownNamedParameters" )" + "argument must be a boolean."); + return false; } - allow_load_extension = allow_extension_v.As()->Value(); + open_config.set_allow_unknown_named_params( + allow_unknown_named_params_v.As()->Value()); } + } - Local timeout_v; - if (!options->Get(env->context(), env->timeout_string()) - .ToLocal(&timeout_v)) { - return; + Local defensive_v; + if (!options->Get(env->context(), env->defensive_string()) + .ToLocal(&defensive_v)) { + return false; + } + if (!defensive_v->IsUndefined()) { + if (!defensive_v->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"options.defensive\" argument must be a boolean."); + return false; } + open_config.set_enable_defensive(defensive_v.As()->Value()); + } - if (!timeout_v->IsUndefined()) { - if (!timeout_v->IsInt32()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"options.timeout\" argument must be an integer."); - return; - } - - open_config.set_timeout(timeout_v.As()->Value()); + // Parse limits option + Local limits_v; + if (!options->Get(env->context(), env->limits_string()).ToLocal(&limits_v)) { + return false; + } + if (!limits_v->IsUndefined()) { + if (!limits_v->IsObject()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), "The \"options.limits\" argument must be an object."); + return false; } - Local read_bigints_v; - if (options->Get(env->context(), env->read_bigints_string()) - .ToLocal(&read_bigints_v)) { - if (!read_bigints_v->IsUndefined()) { - if (!read_bigints_v->IsBoolean()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - R"(The "options.readBigInts" argument must be a boolean.)"); - return; - } - open_config.set_use_big_ints(read_bigints_v.As()->Value()); + Local limits_obj = limits_v.As(); + + // Iterate through known limit names and extract values + for (const auto& [js_name, sqlite_limit_id] : kLimitMapping) { + Local key; + if (!String::NewFromUtf8(env->isolate(), + js_name.data(), + NewStringType::kNormal, + js_name.size()) + .ToLocal(&key)) { + return false; } - } - Local return_arrays_v; - if (options->Get(env->context(), env->return_arrays_string()) - .ToLocal(&return_arrays_v)) { - if (!return_arrays_v->IsUndefined()) { - if (!return_arrays_v->IsBoolean()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - R"(The "options.returnArrays" argument must be a boolean.)"); - return; - } - open_config.set_return_arrays(return_arrays_v.As()->Value()); + Local val; + if (!limits_obj->Get(env->context(), key).ToLocal(&val)) { + return false; } - } - Local allow_bare_named_params_v; - if (options->Get(env->context(), env->allow_bare_named_params_string()) - .ToLocal(&allow_bare_named_params_v)) { - if (!allow_bare_named_params_v->IsUndefined()) { - if (!allow_bare_named_params_v->IsBoolean()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - R"(The "options.allowBareNamedParameters" )" - "argument must be a boolean."); - return; + if (!val->IsUndefined()) { + if (!val->IsInt32()) { + std::string msg = "The \"options.limits." + std::string(js_name) + + "\" argument must be an integer."; + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), msg); + return false; } - open_config.set_allow_bare_named_params( - allow_bare_named_params_v.As()->Value()); - } - } - Local allow_unknown_named_params_v; - if (options->Get(env->context(), env->allow_unknown_named_params_string()) - .ToLocal(&allow_unknown_named_params_v)) { - if (!allow_unknown_named_params_v->IsUndefined()) { - if (!allow_unknown_named_params_v->IsBoolean()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - R"(The "options.allowUnknownNamedParameters" )" - "argument must be a boolean."); - return; + int limit_val = val.As()->Value(); + if (limit_val < 0) { + std::string msg = "The \"options.limits." + std::string(js_name) + + "\" argument must be non-negative."; + THROW_ERR_OUT_OF_RANGE(env->isolate(), msg); + return false; } - open_config.set_allow_unknown_named_params( - allow_unknown_named_params_v.As()->Value()); - } - } - Local defensive_v; - if (!options->Get(env->context(), env->defensive_string()) - .ToLocal(&defensive_v)) { - return; - } - if (!defensive_v->IsUndefined()) { - if (!defensive_v->IsBoolean()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"options.defensive\" argument must be a boolean."); - return; + open_config.set_initial_limit(sqlite_limit_id, limit_val); } - open_config.set_enable_defensive(defensive_v.As()->Value()); - } - - // Parse limits option - Local limits_v; - if (!options->Get(env->context(), env->limits_string()) - .ToLocal(&limits_v)) { - return; } - if (!limits_v->IsUndefined()) { - if (!limits_v->IsObject()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"options.limits\" argument must be an object."); - return; - } - - Local limits_obj = limits_v.As(); - - // Iterate through known limit names and extract values - for (const auto& [js_name, sqlite_limit_id] : kLimitMapping) { - Local key; - if (!String::NewFromUtf8(env->isolate(), - js_name.data(), - NewStringType::kNormal, - js_name.size()) - .ToLocal(&key)) { - return; - } + } + return true; +} +} // namespace - Local val; - if (!limits_obj->Get(env->context(), key).ToLocal(&val)) { - return; - } +void DatabaseSync::New(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (!args.IsConstructCall()) { + THROW_ERR_CONSTRUCT_CALL_REQUIRED(env); + return; + } - if (!val->IsUndefined()) { - if (!val->IsInt32()) { - std::string msg = "The \"options.limits." + std::string(js_name) + - "\" argument must be an integer."; - THROW_ERR_INVALID_ARG_TYPE(env->isolate(), msg); - return; - } + std::optional location = + ValidateDatabasePath(env, args[0], "path"); + if (!location.has_value()) { + return; + } - int limit_val = val.As()->Value(); - if (limit_val < 0) { - std::string msg = "The \"options.limits." + std::string(js_name) + - "\" argument must be non-negative."; - THROW_ERR_OUT_OF_RANGE(env->isolate(), msg); - return; - } + DatabaseOpenConfiguration open_config(std::move(location.value())); + bool open = true; + bool allow_load_extension = false; + if (args.Length() > 1) { + if (!args[1]->IsObject()) { + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"options\" argument must be an object."); + return; + } - open_config.set_initial_limit(sqlite_limit_id, limit_val); - } - } + Local options = args[1].As(); + if (!ParseCommonDatabaseOptions( + env, options, open_config, open, allow_load_extension)) { + return; } } From 78f4e12b5f5d23afca7082966d34502668d72fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Sat, 21 Feb 2026 12:02:09 +0000 Subject: [PATCH 07/15] sqlite: add Database class scaffolding Add a Database class skeleton which is currently only capable of opening and closing a sqlite db connection. Also notably missing is any support for asynchronous operations apart from changes to the close and dispose method signatures. However, the skeleton is capable enough to pass the most basic lifetime tests and therefore useful as a diff target for the asynchronous operations to be added. Re-enable the async database test suites and skip the tests on a case-by -case basis. --- src/node_sqlite.cc | 159 ++++++++++++ src/node_sqlite.h | 26 ++ test/parallel/test-sqlite-database-async.mjs | 246 ++++++++++--------- 3 files changed, 314 insertions(+), 117 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index ed253e2535b5b6..459432bbb7b64b 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -292,6 +292,23 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, int errcode) { } } +inline void RejectWithSQLiteError(Environment* env, + Local resolver, + sqlite3* db) { + resolver + ->Reject(env->context(), + CreateSQLiteError(env->isolate(), db).ToLocalChecked()) + .Check(); +} +inline void RejectWithSQLiteError(Environment* env, + Local resolver, + int errcode) { + resolver + ->Reject(env->context(), + CreateSQLiteError(env->isolate(), errcode).ToLocalChecked()) + .Check(); +} + inline MaybeLocal NullableSQLiteStringToValue(Isolate* isolate, const char* str) { if (str == nullptr) { @@ -3744,6 +3761,143 @@ void Session::Delete() { session_ = nullptr; } +Database::Database(Environment* env, + v8::Local object, + DatabaseOpenConfiguration&& open_config, + bool open, + bool allow_load_extension) + : DatabaseCommon( + env, object, std::move(open_config), allow_load_extension) { + MakeWeak(); + + if (open) { + Open(); + } +} + +Database::~Database() { + if (IsOpen()) { + (void)sqlite3_close_v2(connection_); + connection_ = nullptr; + } +} + +void Database::MemoryInfo(MemoryTracker* tracker) const { + // TODO(BurningEnlightenment): more accurately track the size of all fields + tracker->TrackFieldWithSize( + "open_config", sizeof(open_config_), "DatabaseOpenConfiguration"); +} + +namespace { +v8::Local CreateDatabaseConstructorTemplate( + Environment* env) { + Isolate* isolate = env->isolate(); + + Local tmpl = NewFunctionTemplate(isolate, Database::New); + tmpl->InstanceTemplate()->SetInternalFieldCount( + Database::kInternalFieldCount); + + AddDatabaseCommonMethodsToTemplate(isolate, tmpl); + + SetProtoMethod(isolate, tmpl, "close", Database::Close); + SetProtoAsyncDispose(isolate, tmpl, Database::AsyncDispose); + + Local sqlite_type_key = FIXED_ONE_BYTE_STRING(isolate, "sqlite-type"); + Local sqlite_type_symbol = + v8::Symbol::For(isolate, sqlite_type_key); + Local database_sync_string = + FIXED_ONE_BYTE_STRING(isolate, "node:sqlite-async"); + tmpl->InstanceTemplate()->Set(sqlite_type_symbol, database_sync_string); + + return tmpl; +} +} // namespace + +void Database::New(const v8::FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (!args.IsConstructCall()) { + THROW_ERR_CONSTRUCT_CALL_REQUIRED(env); + return; + } + + std::optional location = + ValidateDatabasePath(env, args[0], "path"); + if (!location.has_value()) { + return; + } + + DatabaseOpenConfiguration open_config(std::move(location.value())); + bool open = true; + bool allow_load_extension = false; + if (args.Length() > 1) { + if (!args[1]->IsObject()) { + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"options\" argument must be an object."); + return; + } + + Local options = args[1].As(); + if (!ParseCommonDatabaseOptions( + env, options, open_config, open, allow_load_extension)) { + return; + } + } + + new Database( + env, args.This(), std::move(open_config), open, allow_load_extension); +} + +void Database::Close(const v8::FunctionCallbackInfo& args) { + Database* db; + ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); + + Environment* env = Environment::GetCurrent(args); + Local context = env->context(); + if (!db->IsOpen()) { + auto resolver = Promise::Resolver::New(context).ToLocalChecked(); + resolver + ->Reject(context, + ERR_INVALID_STATE(env->isolate(), "database is not open")) + .Check(); + args.GetReturnValue().Set(resolver->GetPromise()); + return; + } + + AsyncDispose(args); +} + +void Database::AsyncDispose(const v8::FunctionCallbackInfo& args) { + Database* db; + ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); + + // Disposing is idempotent, so if a close is already in progress or has been + // completed, return the existing promise. + Local close_resolver = + db->object()->GetInternalField(Database::kClosingPromiseSlot).As(); + if (close_resolver->IsPromise()) { + args.GetReturnValue().Set(close_resolver.As()); + return; + } + + Environment* env = Environment::GetCurrent(args); + Local context = env->context(); + auto resolver = Promise::Resolver::New(context).ToLocalChecked(); + + if (!db->IsOpen()) { + resolver->Resolve(context, Undefined(env->isolate())).Check(); + } else if (int errcode = sqlite3_close_v2(db->connection_); + errcode != SQLITE_OK) { + // Note: passing `db->connection_` to sqlite3_extended_errcode after calling + // `sqlite3_close_v2` is not safe. + RejectWithSQLiteError(env, resolver, errcode); + } else { + resolver->Resolve(context, Undefined(env->isolate())).Check(); + } + db->connection_ = nullptr; + db->object()->SetInternalField(Database::kClosingPromiseSlot, resolver); + args.GetReturnValue().Set(resolver->GetPromise()); +} + void DefineConstants(Local target) { NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_OMIT); NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_REPLACE); @@ -3818,6 +3972,11 @@ static void Initialize(Local target, StatementSync::GetConstructorTemplate(env)); SetConstructorFunction( context, target, "Session", Session::GetConstructorTemplate(env)); + SetConstructorFunction( + context, target, "Database", CreateDatabaseConstructorTemplate(env)); + target + ->Set(context, FIXED_ONE_BYTE_STRING(isolate, "Statement"), Null(isolate)) + .Check(); target->Set(context, env->constants_string(), constants).Check(); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 8b3c2a12c56810..0d33fe0df25352 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -448,6 +448,32 @@ class DatabaseSyncLimits : public BaseObject { BaseObjectWeakPtr database_; }; +class Database : public DatabaseCommon { + public: + enum InternalFields { + kClosingPromiseSlot = DatabaseCommon::kInternalFieldCount, + kInternalFieldCount + }; + + Database(Environment* env, + v8::Local object, + DatabaseOpenConfiguration&& open_config, + bool open, + bool allow_load_extension); + void MemoryInfo(MemoryTracker* tracker) const override; + static void New(const v8::FunctionCallbackInfo& args); + static void Close(const v8::FunctionCallbackInfo& args); + static void AsyncDispose(const v8::FunctionCallbackInfo& args); + + SET_MEMORY_INFO_NAME(Database) + SET_SELF_SIZE(Database) + + private: + ~Database() override; + + friend class StatementExecutionHelper; +}; + } // namespace sqlite } // namespace node diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index 3a1322604f5547..ee6bd25c4223f2 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -1,5 +1,4 @@ -import { skip, skipIfSQLiteMissing } from '../common/index.mjs'; -skip(); +import { skipIfSQLiteMissing } from '../common/index.mjs'; import tmpdir from '../common/tmpdir.js'; import { existsSync } from 'node:fs'; import { suite, test } from 'node:test'; @@ -87,18 +86,22 @@ suite('Database() constructor', () => { }); }); - test('is not read-only by default', async (t) => { + test.skip('is not read-only by default', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath); await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); }); - test('is read-only if readOnly is set', async (t) => { + test.skip('is read-only if readOnly is set', async (t) => { const dbPath = nextDb(); { + // TODO: use `await using` once it's supported in test files const db = new Database(dbPath); - await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); - db.close(); + try { + await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + } finally { + await db.close(); + } } { const db = new Database(dbPath, { readOnly: true }); @@ -118,14 +121,14 @@ suite('Database() constructor', () => { }); }); - test('enables foreign key constraints by default', async (t) => { + test.skip('enables foreign key constraints by default', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath); await db.exec(` CREATE TABLE foo (id INTEGER PRIMARY KEY); CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); `); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); await t.assert.rejects( db.exec('INSERT INTO bar (foo_id) VALUES (1)'), { @@ -134,14 +137,14 @@ suite('Database() constructor', () => { }); }); - test('allows disabling foreign key constraints', async (t) => { + test.skip('allows disabling foreign key constraints', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { enableForeignKeyConstraints: false }); await db.exec(` CREATE TABLE foo (id INTEGER PRIMARY KEY); CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); `); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); await db.exec('INSERT INTO bar (foo_id) VALUES (1)'); }); @@ -154,20 +157,20 @@ suite('Database() constructor', () => { }); }); - test('disables double-quoted string literals by default', async (t) => { + test.skip('disables double-quoted string literals by default', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); await t.assert.rejects(db.exec('SELECT "foo";'), { code: 'ERR_SQLITE_ERROR', message: /no such column: "?foo"?/, }); }); - test('allows enabling double-quoted string literals', async (t) => { + test.skip('allows enabling double-quoted string literals', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { enableDoubleQuotedStringLiterals: true }); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); await db.exec('SELECT "foo";'); }); @@ -180,10 +183,10 @@ suite('Database() constructor', () => { }); }); - test('allows reading big integers', async (t) => { + test.skip('allows reading big integers', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { readBigInts: true }); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec(` CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT; @@ -210,10 +213,10 @@ suite('Database() constructor', () => { }); }); - test('allows returning arrays', async (t) => { + test.skip('allows returning arrays', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { returnArrays: true }); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec(` CREATE TABLE data(key INTEGER PRIMARY KEY, val TEXT) STRICT; INSERT INTO data (key, val) VALUES (1, 'one'); @@ -234,10 +237,10 @@ suite('Database() constructor', () => { }); }); - test('throws if bare named parameters are used when option is false', async (t) => { + test.skip('throws if bare named parameters are used when option is false', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { allowBareNamedParameters: false }); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec( 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' ); @@ -261,10 +264,10 @@ suite('Database() constructor', () => { }); }); - test('allows unknown named parameters', async (t) => { + test.skip('allows unknown named parameters', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { allowUnknownNamedParameters: true }); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec( 'CREATE TABLE data(key INTEGER, val INTEGER) STRICT;' ); @@ -280,11 +283,11 @@ suite('Database() constructor', () => { test('has sqlite-type symbol property', (t) => { const dbPath = nextDb(); - const db = new Database(dbPath); - t.after(() => { db.close(); }); + const db = new Database(dbPath, { open: false }); + t.after(async () => { await db[Symbol.asyncDispose](); }); const sqliteTypeSymbol = Symbol.for('sqlite-type'); - t.assert.strictEqual(db[sqliteTypeSymbol], 'node:sqlite'); + t.assert.strictEqual(db[sqliteTypeSymbol], 'node:sqlite-async'); }); }); @@ -292,7 +295,7 @@ suite('Database.prototype.open()', () => { test('opens a database connection', (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { open: false }); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); t.assert.strictEqual(db.isOpen, false); t.assert.strictEqual(existsSync(dbPath), false); @@ -303,7 +306,7 @@ suite('Database.prototype.open()', () => { test('throws if database is already open', (t) => { const db = new Database(nextDb(), { open: false }); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); t.assert.strictEqual(db.isOpen, false); db.open(); @@ -319,32 +322,111 @@ suite('Database.prototype.open()', () => { }); suite('Database.prototype.close()', () => { - test('closes an open database connection', (t) => { + test('closes an open database connection', async (t) => { const db = new Database(nextDb()); t.assert.strictEqual(db.isOpen, true); - t.assert.strictEqual(db.close(), undefined); + t.assert.strictEqual(await db.close(), undefined); t.assert.strictEqual(db.isOpen, false); }); - test('throws if database is not open', (t) => { + test('throws if database is not open', async (t) => { + const db = new Database(nextDb(), { open: false }); + + t.assert.strictEqual(db.isOpen, false); + await t.assert.rejects(db.close(), { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + t.assert.strictEqual(db.isOpen, false); + }); + + // These tests need to be rewritten to account for the fact that close() is + // async, so instead of rejecting pending operations, it will wait for them to + // finish before closing the database. However, we can still test that new + // operations are rejected once the database is closing. + test.skip('rejects pending operations when database is closed', async (t) => { + const db = new Database(':memory:'); + await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)'); + const pendingOp = db.exec('INSERT INTO test (value) VALUES (\'test\')'); + await db.close(); + + await t.assert.rejects(pendingOp, { + code: 'ERR_INVALID_STATE', + message: /database is closing/, + }); + }); + + test.skip('rejects multiple pending operations when database is closed', async (t) => { + const db = new Database(':memory:'); + await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)'); + const ops = [ + db.exec('INSERT INTO test (value) VALUES (\'test1\')'), + db.exec('INSERT INTO test (value) VALUES (\'test2\')'), + db.exec('INSERT INTO test (value) VALUES (\'test3\')'), + ]; + + await db.close(); + + const results = await Promise.allSettled(ops); + t.assert.partialDeepStrictEqual( + results, + ops.map(() => ({ + status: 'rejected', + reason: { + code: 'ERR_INVALID_STATE', + message: /database is closing/, + }, + })) + ); + t.assert.strictEqual(results.length, ops.length); + }); +}); + +suite('Database.prototype[Symbol.asyncDispose]()', () => { + test('closes an open database connection', async (t) => { + const db = new Database(nextDb()); + + t.assert.strictEqual(db.isOpen, true); + t.assert.strictEqual(await db[Symbol.asyncDispose](), undefined); + t.assert.strictEqual(db.isOpen, false); + }); + test('does not throw if the database is not open', async (t) => { + const db = new Database(nextDb(), { open: false }); + + t.assert.strictEqual(db.isOpen, false); + t.assert.strictEqual(await db[Symbol.asyncDispose](), undefined); + t.assert.strictEqual(db.isOpen, false); + }); + test.skip('prevents a database from being opened after disposal', async (t) => { const db = new Database(nextDb(), { open: false }); + t.assert.strictEqual(db.isOpen, false); + t.assert.strictEqual(await db[Symbol.asyncDispose](), undefined); t.assert.strictEqual(db.isOpen, false); t.assert.throws(() => { - db.close(); + db.open(); }, { code: 'ERR_INVALID_STATE', - message: /database is not open/, + message: /database is disposed/, }); t.assert.strictEqual(db.isOpen, false); }); + test('is idempotent', async (t) => { + const db = new Database(nextDb()); + + t.assert.strictEqual(db.isOpen, true); + t.assert.strictEqual(await db[Symbol.asyncDispose](), undefined); + t.assert.strictEqual(db.isOpen, false); + t.assert.strictEqual(await db[Symbol.asyncDispose](), undefined); + t.assert.strictEqual(db.isOpen, false); + }); }); -suite('Database.prototype.prepare()', () => { +suite.skip('Database.prototype.prepare()', () => { test('returns a prepared statement', (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const stmt = db.prepare('CREATE TABLE webstorage(key TEXT)'); t.assert.ok(stmt instanceof Statement); }); @@ -362,7 +444,7 @@ suite('Database.prototype.prepare()', () => { test('throws if sql is not a string', (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); t.assert.throws(() => { db.prepare(); @@ -373,10 +455,10 @@ suite('Database.prototype.prepare()', () => { }); }); -suite('Database.prototype.exec()', () => { +suite.skip('Database.prototype.exec()', () => { test('executes SQL', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const result = await db.exec(` CREATE TABLE data( key INTEGER PRIMARY KEY, @@ -395,7 +477,7 @@ suite('Database.prototype.exec()', () => { test('reports errors from SQLite', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); await t.assert.rejects( db.exec('CREATE TABLEEEE'), @@ -427,7 +509,7 @@ suite('Database.prototype.exec()', () => { test('throws if sql is not a string', (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); t.assert.throws(() => { db.exec(); @@ -438,7 +520,7 @@ suite('Database.prototype.exec()', () => { }); }); -suite('Database.prototype.isTransaction', () => { +suite.skip('Database.prototype.isTransaction', () => { test('correctly detects a committed transaction', async (t) => { const db = new Database(':memory:'); @@ -475,7 +557,7 @@ suite('Database.prototype.isTransaction', () => { }); }); -suite('Database.prototype.location()', () => { +suite.skip('Database.prototype.location()', () => { test('throws if database is not open', (t) => { const db = new Database(nextDb(), { open: false }); @@ -489,7 +571,7 @@ suite('Database.prototype.location()', () => { test('throws if provided dbName is not string', (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); t.assert.throws(() => { db.location(null); @@ -507,7 +589,7 @@ suite('Database.prototype.location()', () => { test('returns db path when connected to a persistent database', (t) => { const dbPath = nextDb(); const db = new Database(dbPath); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); t.assert.strictEqual(db.location(), dbPath); }); @@ -515,9 +597,9 @@ suite('Database.prototype.location()', () => { const dbPath = nextDb(); const otherPath = nextDb(); const db = new Database(dbPath); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const other = new Database(dbPath); - t.after(() => { other.close(); }); + t.after(async () => { await other.close(); }); // Adding this escape because the test with unusual chars have a single quote which breaks the query const escapedPath = otherPath.replace("'", "''"); @@ -527,48 +609,10 @@ suite('Database.prototype.location()', () => { }); }); -suite('Async mode restrictions', () => { - test('throws when defining a custom function', (t) => { - const db = new Database(':memory:'); - t.after(() => { db.close(); }); - - t.assert.throws(() => { - db.function('test', () => 1); - }, { - code: 'ERR_INVALID_STATE', - message: /Custom functions are not supported in async mode/, - }); - }); - - test('throws when defining an aggregate function', (t) => { - const db = new Database(':memory:'); - t.after(() => { db.close(); }); - - t.assert.throws(() => { - db.aggregate('test', { start: 0, step: (acc) => acc }); - }, { - code: 'ERR_INVALID_STATE', - message: /Aggregate functions are not supported in async mode/, - }); - }); - - test('throws when setting an authorizer callback', (t) => { - const db = new Database(':memory:'); - t.after(() => { db.close(); }); - - t.assert.throws(() => { - db.setAuthorizer(() => 0); - }, { - code: 'ERR_INVALID_STATE', - message: /Authorizer callbacks are not supported in async mode/, - }); - }); -}); - -suite('Async operation ordering', () => { +suite.skip('Async operation ordering', () => { test('executes operations sequentially per database', async (t) => { const db = new Database(':memory:'); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, seq INTEGER)'); // Launch multiple operations concurrently @@ -623,35 +667,3 @@ suite('Async operation ordering', () => { ); }); }); - -suite('Database.prototype.close', () => { - test('rejects pending operations when database is closed', async (t) => { - const db = new Database(':memory:'); - await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)'); - const pendingOp = db.exec('INSERT INTO test (value) VALUES (\'test\')'); - db.close(); - - await t.assert.rejects(pendingOp, { - code: 'ERR_INVALID_STATE', - message: /database is closing/, - }); - }); - - test('rejects multiple pending operations when database is closed', async (t) => { - const db = new Database(':memory:'); - await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)'); - const ops = [ - db.exec('INSERT INTO test (value) VALUES (\'test1\')'), - db.exec('INSERT INTO test (value) VALUES (\'test2\')'), - db.exec('INSERT INTO test (value) VALUES (\'test3\')'), - ]; - - db.close(); - - const results = await Promise.allSettled(ops); - for (const result of results) { - t.assert.strictEqual(result.status, 'rejected'); - t.assert.strictEqual(result.reason.code, 'ERR_INVALID_STATE'); - } - }); -}); From 82c136f6095ec19c1f425956fa58cdc11dc769da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 26 Feb 2026 06:56:02 +0000 Subject: [PATCH 08/15] sqlite: implement batched async operation scheduling Trade a bit of db operation latency for throughput by batching db operations together before scheduling their execution on the thead pool. Completion notifications are currently delivered eagerly through an additional `uv_async` handle, but this is not strictly necessary for the chosen implementation strategy and might get removed again. Some care has been taken to not criss-cross memory allocations and de- allocations between threads as modern modern allocators tend to perform worse in such scenarios. --- src/node_sqlite.cc | 524 ++++++++++++++++++- src/node_sqlite.h | 28 +- test/parallel/test-sqlite-database-async.mjs | 63 +-- 3 files changed, 557 insertions(+), 58 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 459432bbb7b64b..f4f502dc715615 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -11,11 +11,14 @@ #include "sqlite3.h" #include "threadpoolwork-inl.h" #include "util-inl.h" +#include "uv.h" #include #include #include #include +#include +#include namespace node { namespace sqlite { @@ -384,7 +387,7 @@ class CustomAggregate { static inline void xStepBase(sqlite3_context* ctx, int argc, sqlite3_value** argv, - Global CustomAggregate::*mptr) { + Global CustomAggregate::* mptr) { CustomAggregate* self = static_cast(sqlite3_user_data(ctx)); Environment* env = self->env_; @@ -3761,25 +3764,435 @@ void Session::Delete() { session_ = nullptr; } +namespace { +class OperationBase { + public: + Local GetResolver(Isolate* isolate) const { + return resolver_.Get(isolate); + } + + protected: + explicit OperationBase(Global resolver) + : resolver_(std::move(resolver)) {} + + Global resolver_; +}; + +class alignas(64) OperationResult { + class Rejected { + public: + static Rejected ErrorCode(int error_code, + const char* error_message = nullptr) { + const char* error_description = sqlite3_errstr(error_code); + return Rejected{ + error_code, + error_message != nullptr ? error_message : std::pmr::string{}, + error_description != nullptr ? error_description : std::pmr::string{}, + }; + } + static Rejected LastError(sqlite3* connection) { + int error_code = sqlite3_extended_errcode(connection); + const char* error_message = sqlite3_errmsg(connection); + return Rejected::ErrorCode(error_code, error_message); + } + + void Connect(Isolate* isolate, + Local context, + Promise::Resolver* resolver) const { + // Use ToLocalChecked here because we are trying to report a failure and + // failing to report a failure would be worse than crashing as it would be + // an API contract violation. + Local error_message = + String::NewFromUtf8(isolate, + error_message_.data(), + NewStringType::kNormal, + static_cast(std::ssize(error_message_))) + .ToLocalChecked(); + Local error_description = + String::NewFromUtf8(isolate, + error_description_.data(), + NewStringType::kInternalized, + static_cast(std::ssize(error_description_))) + .ToLocalChecked(); + + Local error = + Exception::Error(error_message)->ToObject(context).ToLocalChecked(); + error + ->Set(context, + FIXED_ONE_BYTE_STRING(isolate, "code"), + FIXED_ONE_BYTE_STRING(isolate, "ERR_SQLITE_ERROR")) + .Check(); + error + ->Set(context, + FIXED_ONE_BYTE_STRING(isolate, "errcode"), + Integer::New(isolate, error_code_)) + .Check(); + error + ->Set(context, + FIXED_ONE_BYTE_STRING(isolate, "errstr"), + error_description) + .Check(); + + resolver->Reject(context, error).Check(); + } + + private: + Rejected(int error_code, + std::pmr::string error_message, + std::pmr::string error_description) + : error_message_(std::move(error_message)), + error_description_(std::move(error_description)), + error_code_(error_code) {} + + std::pmr::string error_message_; + std::pmr::string error_description_; + int error_code_; + }; + class Void { + public: + void Connect(Isolate* isolate, + Local context, + Promise::Resolver* resolver) const { + resolver->Resolve(context, Undefined(isolate)).Check(); + } + }; + + using variant_type = std::variant; + + public: + static OperationResult RejectErrorCode(OperationBase* origin, + int error_code) { + return OperationResult{origin, Rejected::ErrorCode(error_code)}; + } + static OperationResult RejectLastError(OperationBase* origin, + sqlite3* connection) { + return OperationResult{origin, Rejected::LastError(connection)}; + } + static OperationResult ResolveVoid(OperationBase* origin) { + return OperationResult{origin, Void{}}; + } + + template + requires std::constructible_from + OperationResult(OperationBase* origin, T&& value) + : origin_(origin), result_(std::forward(value)) {} + + void Connect(Isolate* isolate) const { + Local context = isolate->GetCurrentContext(); + Local resolver = origin_->GetResolver(isolate); + std::visit( + [isolate, &context, &resolver](auto&& value) { + value.Connect(isolate, context, *resolver); + }, + result_); + } + + private: + OperationBase* origin_ = nullptr; + variant_type result_; +}; + +class ExecOperation : private OperationBase { + public: + ExecOperation(Global&& resolver, std::pmr::string&& sql) + : OperationBase(std::move(resolver)), sql_(std::move(sql)) {} + + OperationResult operator()(sqlite3* connection) { + int error_code = + sqlite3_exec(connection, sql_.c_str(), nullptr, nullptr, nullptr); + return error_code == SQLITE_OK + ? OperationResult::ResolveVoid(this) + : OperationResult::RejectLastError(this, connection); + } + + private: + std::pmr::string sql_; +}; + +class CloseOperation : private OperationBase { + public: + explicit CloseOperation(Global&& resolver) + : OperationBase(std::move(resolver)) {} + + OperationResult operator()(sqlite3* connection) { + // TODO(BurningEnlightenment): close the associated statements + int error_code = sqlite3_close(connection); + // Busy means that we failed to cleanup associated statements, i.e. we + // failed to maintain the Database object invariants. + CHECK_NE(error_code, SQLITE_BUSY); + DCHECK_NE(error_code, SQLITE_MISUSE); + return error_code == SQLITE_OK + ? OperationResult::ResolveVoid(this) + : OperationResult::RejectErrorCode(this, error_code); + } +}; + +using Operation = std::variant; + +enum class QueuePushResult { + kQueueFull = -1, + kSuccess, + kLastSlot, +}; +} // namespace + +class DatabaseOperationQueue { + public: + explicit DatabaseOperationQueue(size_t capacity, + std::pmr::memory_resource* memory_resource) + : operations_(memory_resource), results_(memory_resource) { + operations_.reserve(capacity); + results_.reserve(capacity); + } + + [[nodiscard]] QueuePushResult Push(Operation operation) { + if (operations_.capacity() == operations_.size()) { + return QueuePushResult::kQueueFull; + } + operations_.push_back(std::move(operation)); + return operations_.size() == operations_.capacity() + ? QueuePushResult::kLastSlot + : QueuePushResult::kSuccess; + } + template + requires std::constructible_from, + Global&&, + Args&&...> + [[nodiscard]] QueuePushResult PushEmplace(Isolate* isolate, + Local resolver, + Args&&... args) { + return Push(Operation{std::in_place_type, + Global{isolate, resolver}, + std::forward(args)...}); + } + void Push(OperationResult result) { + Mutex::ScopedLock lock{results_mutex_}; + CHECK_LT(results_.size(), results_.capacity()); + results_.push_back(std::move(result)); + } + + Operation* PopOperation() { + if (operations_.size() == pending_index_) { + return nullptr; + } + return &operations_[pending_index_++]; + } + std::span PopResults() { + Mutex::ScopedLock lock{results_mutex_}; + if (results_.size() == completed_index_) { + return {}; + } + return std::span{results_}.subspan( + std::exchange(completed_index_, results_.size())); + } + + private: + std::pmr::vector operations_; + std::pmr::vector results_; + size_t pending_index_ = 0; + size_t completed_index_ = 0; + Mutex results_mutex_; +}; + +namespace { +using uv_async_deleter = decltype([](uv_async_t* handle) -> void { + uv_close(reinterpret_cast(handle), + [](uv_handle_t* handle) -> void { + delete reinterpret_cast(handle); + }); +}); +using unique_async_handle = std::unique_ptr; +unique_async_handle make_unique_async_handle(uv_loop_t* loop, + uv_async_cb callback) { + auto* handle = new uv_async_t; + if (uv_async_init(loop, handle, callback) != 0) { + delete handle; + return nullptr; + } + return unique_async_handle{handle}; +} +} // namespace + +class DatabaseOperationExecutor final : private ThreadPoolWork { + public: + DatabaseOperationExecutor(Environment* env, sqlite3* connection) + : ThreadPoolWork(env, "node_sqlite3.OperationExecutor"), + async_completion_(), + connection_(connection) {} + + bool is_working() const { return current_batch_ != nullptr; } + + void ScheduleBatch(std::unique_ptr batch) { + CHECK_NOT_NULL(batch); + auto batch_ptr = batch.get(); + batches_.push(std::move(batch)); + if (!is_working()) { + async_completion_ = make_unique_async_handle(env()->event_loop(), + &AsyncCompletionCallback); + async_completion_->data = this; + DatabaseOperationExecutor::ScheduleWork(batch_ptr); + } + } + + void Dispose() { + CHECK(is_working()); + disposing_ = true; + } + + private: + void DoThreadPoolWork() override { + // Use local copies of member variables to avoid false sharing. + sqlite3* connection = connection_; + DatabaseOperationQueue* batch = current_batch_; + for (Operation* maybe_op = batch->PopOperation(); maybe_op != nullptr; + maybe_op = batch->PopOperation()) { + auto op_result = std::visit( + [connection](auto&& op) -> OperationResult { return op(connection); }, + *maybe_op); + batch->Push(std::move(op_result)); + CHECK_EQ(uv_async_send(async_completion_.get()), 0); + } + } + void AfterThreadPoolWork(int status) override { + CHECK_EQ(status, 0); // Currently the cancellation API is not exposed. + AsyncCompletionCallback(); + current_batch_ = nullptr; + batches_.pop(); + if (!batches_.empty()) { + DatabaseOperationExecutor::ScheduleWork(batches_.front().get()); + } else if (disposing_) { + // The executor is being disposed and there are no more batches to + // process, so we can safely delete it. + delete this; + } else { + // Don't keep the event loop active while there are no batches to process. + async_completion_.reset(); + } + } + static void AsyncCompletionCallback(uv_async_t* handle) { + if (uv_is_closing(reinterpret_cast(handle))) { + return; + } + auto* self = static_cast(handle->data); + // Safe guard against a race between the async_completion and the after_work + // callback; given the current libuv implementation this should not be + // possible, but it's not a documented API guarantee. + if (self->current_batch_ != nullptr) { + self->AsyncCompletionCallback(); + } + } + void AsyncCompletionCallback() { + const auto& results = current_batch_->PopResults(); + if (results.empty()) { + return; + } + Isolate* isolate = env()->isolate(); + HandleScope handle_scope(isolate); + + for (OperationResult& result : results) { + result.Connect(isolate); + } + } + void ScheduleWork(DatabaseOperationQueue* batch) { + CHECK_NOT_NULL(batch); + CHECK_NULL(current_batch_); + current_batch_ = batch; + ThreadPoolWork::ScheduleWork(); + } + + std::queue> batches_; + bool disposing_ = false; + + // These _must_ only be written to while is_working() is false or from within + // AfterThreadPoolWork. Violating this invariant would introduce to data races + // and therefore undefined behavior. + unique_async_handle async_completion_; + sqlite3* connection_ = nullptr; + DatabaseOperationQueue* current_batch_ = nullptr; +}; + +void Database::PrepareNextBatch() { + CHECK_NULL(next_batch_); + next_batch_ = std::make_unique( + kDefaultBatchSize, std::pmr::get_default_resource()); + + // TODO(BurningEnlightenment): Do I need to retain a BaseObjectPtr? + env()->isolate()->EnqueueMicrotask( + [](void* self) -> void { + CHECK_NOT_NULL(self); + static_cast(self)->ProcessNextBatch(); + }, + this); +} +void Database::ProcessNextBatch() { + if (next_batch_ != nullptr) { + executor_->ScheduleBatch(std::move(next_batch_)); + } +} +template +void Database::Schedule(v8::Isolate* isolate, + v8::Local resolver, + Args&&... args) { + if (next_batch_ == nullptr) { + PrepareNextBatch(); + } + QueuePushResult r = next_batch_->PushEmplace( + isolate, resolver, std::forward(args)...); + if (r == QueuePushResult::kSuccess) [[likely]] { + return; + } + // Batch is full, schedule it for execution. + ProcessNextBatch(); + // With the current set of invariants next_batch_ should never be full + CHECK_EQ(r, QueuePushResult::kLastSlot); +} + +template +Local Database::Schedule(Args&&... args) { + Isolate* isolate = env()->isolate(); + Local context = isolate->GetCurrentContext(); + auto resolver = Promise::Resolver::New(context).ToLocalChecked(); + + Database::Schedule(isolate, resolver, std::forward(args)...); + + return resolver->GetPromise(); +} + Database::Database(Environment* env, v8::Local object, DatabaseOpenConfiguration&& open_config, bool open, bool allow_load_extension) - : DatabaseCommon( - env, object, std::move(open_config), allow_load_extension) { + : DatabaseCommon(env, object, std::move(open_config), allow_load_extension), + executor_(nullptr), + next_batch_(nullptr) { MakeWeak(); if (open) { - Open(); + Database::Open(); } } Database::~Database() { - if (IsOpen()) { + if (!IsOpen()) [[likely]] { + return; + } + if (executor_->is_working()) { + HandleScope handle_scope(env()->isolate()); + (void)AsyncDisposeImpl(); + } else { (void)sqlite3_close_v2(connection_); - connection_ = nullptr; } + + env()->SetImmediate([](Environment* env) { + HandleScope handle_scope(env->isolate()); + + THROW_ERR_INVALID_STATE( + env->isolate(), + "Database was not properly closed before being garbage collected. " + "Please call and await db.close() to close the database connection."); + }); } void Database::MemoryInfo(MemoryTracker* tracker) const { @@ -3801,6 +4214,7 @@ v8::Local CreateDatabaseConstructorTemplate( SetProtoMethod(isolate, tmpl, "close", Database::Close); SetProtoAsyncDispose(isolate, tmpl, Database::AsyncDispose); + SetProtoMethod(isolate, tmpl, "exec", Database::Exec); Local sqlite_type_key = FIXED_ONE_BYTE_STRING(isolate, "sqlite-type"); Local sqlite_type_symbol = @@ -3847,6 +4261,60 @@ void Database::New(const v8::FunctionCallbackInfo& args) { env, args.This(), std::move(open_config), open, allow_load_extension); } +bool Database::Open() { + if (IsDisposed()) { + THROW_ERR_INVALID_STATE(env(), "database is disposed"); + return false; + } + auto open_result = DatabaseCommon::Open(); + if (open_result) { + executor_ = std::make_unique(env(), connection_); + } + return open_result; +} + +bool Database::IsDisposed() const { + return object() + ->GetInternalField(Database::kDisposePromiseSlot) + .As() + ->IsPromise(); +} + +v8::Local Database::EnterDisposedStateSync() { + Isolate* isolate = env()->isolate(); + Local context = isolate->GetCurrentContext(); + auto disposed = Promise::Resolver::New(context).ToLocalChecked(); + disposed->Resolve(context, Undefined(isolate)).Check(); + + auto dispose_promise = disposed->GetPromise(); + object()->SetInternalField(Database::kDisposePromiseSlot, dispose_promise); + return dispose_promise; +} + +Local Database::AsyncDisposeImpl() { + Isolate* isolate = env()->isolate(); + Local context = isolate->GetCurrentContext(); + auto resolver = Promise::Resolver::New(context).ToLocalChecked(); + + // We can't use Schedule here, because Schedule queues a MicroTask which + // would try to access the Database object after destruction if this is + // called from the destructor. + if (next_batch_ == nullptr) { + next_batch_ = std::make_unique( + 1U, std::pmr::get_default_resource()); + } + CHECK_NE(next_batch_->PushEmplace(isolate, resolver), + QueuePushResult::kQueueFull); + executor_->ScheduleBatch(std::move(next_batch_)); + executor_.release()->Dispose(); + + connection_ = nullptr; + + auto dispose_promise = resolver->GetPromise(); + object()->SetInternalField(Database::kDisposePromiseSlot, dispose_promise); + return dispose_promise; +} + void Database::Close(const v8::FunctionCallbackInfo& args) { Database* db; ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); @@ -3854,6 +4322,9 @@ void Database::Close(const v8::FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Local context = env->context(); if (!db->IsOpen()) { + if (!db->IsDisposed()) { + db->EnterDisposedStateSync(); + } auto resolver = Promise::Resolver::New(context).ToLocalChecked(); resolver ->Reject(context, @@ -3872,30 +4343,35 @@ void Database::AsyncDispose(const v8::FunctionCallbackInfo& args) { // Disposing is idempotent, so if a close is already in progress or has been // completed, return the existing promise. - Local close_resolver = - db->object()->GetInternalField(Database::kClosingPromiseSlot).As(); - if (close_resolver->IsPromise()) { - args.GetReturnValue().Set(close_resolver.As()); + Local maybe_dispose_promise = + db->object()->GetInternalField(Database::kDisposePromiseSlot).As(); + if (maybe_dispose_promise->IsPromise()) { + args.GetReturnValue().Set(maybe_dispose_promise.As()); return; } + if (!db->IsOpen()) { + args.GetReturnValue().Set(db->EnterDisposedStateSync()); + return; + } + + args.GetReturnValue().Set(db->AsyncDisposeImpl()); +} +void Database::Exec(const v8::FunctionCallbackInfo& args) { + Database* db; + // TODO(BurningEnlightenment): these should be rejections + ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); - Local context = env->context(); - auto resolver = Promise::Resolver::New(context).ToLocalChecked(); + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); - if (!db->IsOpen()) { - resolver->Resolve(context, Undefined(env->isolate())).Check(); - } else if (int errcode = sqlite3_close_v2(db->connection_); - errcode != SQLITE_OK) { - // Note: passing `db->connection_` to sqlite3_extended_errcode after calling - // `sqlite3_close_v2` is not safe. - RejectWithSQLiteError(env, resolver, errcode); - } else { - resolver->Resolve(context, Undefined(env->isolate())).Check(); + if (!args[0]->IsString()) { + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"sql\" argument must be a string."); + return; } - db->connection_ = nullptr; - db->object()->SetInternalField(Database::kClosingPromiseSlot, resolver); - args.GetReturnValue().Set(resolver->GetPromise()); + Utf8Value sql(env->isolate(), args[0].As()); + args.GetReturnValue().Set( + db->Schedule(std::pmr::string(*sql, sql.length()))); } void DefineConstants(Local target) { diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 0d33fe0df25352..cfcae7c42745ca 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -181,7 +181,7 @@ class DatabaseCommon : public BaseObject { protected: ~DatabaseCommon() override = default; - bool Open(); + virtual bool Open(); DatabaseOpenConfiguration open_config_; sqlite3* connection_ = nullptr; @@ -448,10 +448,13 @@ class DatabaseSyncLimits : public BaseObject { BaseObjectWeakPtr database_; }; -class Database : public DatabaseCommon { +class DatabaseOperationExecutor; +class DatabaseOperationQueue; + +class Database final : public DatabaseCommon { public: enum InternalFields { - kClosingPromiseSlot = DatabaseCommon::kInternalFieldCount, + kDisposePromiseSlot = DatabaseCommon::kInternalFieldCount, kInternalFieldCount }; @@ -464,6 +467,7 @@ class Database : public DatabaseCommon { static void New(const v8::FunctionCallbackInfo& args); static void Close(const v8::FunctionCallbackInfo& args); static void AsyncDispose(const v8::FunctionCallbackInfo& args); + static void Exec(const v8::FunctionCallbackInfo& args); SET_MEMORY_INFO_NAME(Database) SET_SELF_SIZE(Database) @@ -471,6 +475,24 @@ class Database : public DatabaseCommon { private: ~Database() override; + bool Open() override; + bool IsDisposed() const; + v8::Local EnterDisposedStateSync(); + v8::Local AsyncDisposeImpl(); + void PrepareNextBatch(); + void ProcessNextBatch(); + template + [[nodiscard]] v8::Local Schedule(Args&&... args); + template + void Schedule(v8::Isolate* isolate, + v8::Local resolver, + Args&&... args); + + std::unique_ptr executor_; + std::unique_ptr next_batch_; + + static constexpr int kDefaultBatchSize = 31; + friend class StatementExecutionHelper; }; diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index ee6bd25c4223f2..5fd874f506dc51 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -13,7 +13,7 @@ function nextDb() { return join(tmpdir.path, `database-${cnt++}.db`); } -suite('Database() constructor', () => { +suite('Database() constructor', { timeout: 1000 }, () => { test('throws if called without new', (t) => { t.assert.throws(() => { Database(); @@ -32,6 +32,15 @@ suite('Database() constructor', () => { }); }); + test('throws if the database URL does not have the file: scheme', (t) => { + t.assert.throws(() => { + new Database(new URL('http://example.com')); + }, { + code: 'ERR_INVALID_URL_SCHEME', + message: 'The URL must be of scheme file:', + }); + }); + test('throws if the database location as Buffer contains null bytes', (t) => { t.assert.throws(() => { new Database(Buffer.from('l\0cation')); @@ -86,13 +95,14 @@ suite('Database() constructor', () => { }); }); - test.skip('is not read-only by default', async (t) => { + test('is not read-only by default', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath); + t.after(async () => { await db[Symbol.asyncDispose](); }); await db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); }); - test.skip('is read-only if readOnly is set', async (t) => { + test('is read-only if readOnly is set', async (t) => { const dbPath = nextDb(); { // TODO: use `await using` once it's supported in test files @@ -105,6 +115,7 @@ suite('Database() constructor', () => { } { const db = new Database(dbPath, { readOnly: true }); + t.after(async () => { await db[Symbol.asyncDispose](); }); await t.assert.rejects(db.exec('CREATE TABLE bar (id INTEGER PRIMARY KEY)'), { code: 'ERR_SQLITE_ERROR', message: /attempt to write a readonly database/, @@ -121,14 +132,14 @@ suite('Database() constructor', () => { }); }); - test.skip('enables foreign key constraints by default', async (t) => { + test('enables foreign key constraints by default', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath); + t.after(async () => { await db[Symbol.asyncDispose](); }); await db.exec(` CREATE TABLE foo (id INTEGER PRIMARY KEY); CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); `); - t.after(async () => { await db.close(); }); await t.assert.rejects( db.exec('INSERT INTO bar (foo_id) VALUES (1)'), { @@ -137,14 +148,14 @@ suite('Database() constructor', () => { }); }); - test.skip('allows disabling foreign key constraints', async (t) => { + test('allows disabling foreign key constraints', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { enableForeignKeyConstraints: false }); + t.after(async () => { await db.close(); }); await db.exec(` CREATE TABLE foo (id INTEGER PRIMARY KEY); CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); `); - t.after(async () => { await db.close(); }); await db.exec('INSERT INTO bar (foo_id) VALUES (1)'); }); @@ -157,7 +168,7 @@ suite('Database() constructor', () => { }); }); - test.skip('disables double-quoted string literals by default', async (t) => { + test('disables double-quoted string literals by default', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath); t.after(async () => { await db.close(); }); @@ -167,7 +178,7 @@ suite('Database() constructor', () => { }); }); - test.skip('allows enabling double-quoted string literals', async (t) => { + test('allows enabling double-quoted string literals', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { enableDoubleQuotedStringLiterals: true }); t.after(async () => { await db.close(); }); @@ -284,14 +295,13 @@ suite('Database() constructor', () => { test('has sqlite-type symbol property', (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { open: false }); - t.after(async () => { await db[Symbol.asyncDispose](); }); const sqliteTypeSymbol = Symbol.for('sqlite-type'); t.assert.strictEqual(db[sqliteTypeSymbol], 'node:sqlite-async'); }); }); -suite('Database.prototype.open()', () => { +suite('Database.prototype.open()', { timeout: 1000 }, () => { test('opens a database connection', (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { open: false }); @@ -321,7 +331,7 @@ suite('Database.prototype.open()', () => { }); }); -suite('Database.prototype.close()', () => { +suite('Database.prototype.close()', { timeout: 1000 }, () => { test('closes an open database connection', async (t) => { const db = new Database(nextDb()); @@ -383,7 +393,7 @@ suite('Database.prototype.close()', () => { }); }); -suite('Database.prototype[Symbol.asyncDispose]()', () => { +suite('Database.prototype[Symbol.asyncDispose]()', { timeout: 1000 }, () => { test('closes an open database connection', async (t) => { const db = new Database(nextDb()); @@ -398,7 +408,7 @@ suite('Database.prototype[Symbol.asyncDispose]()', () => { t.assert.strictEqual(await db[Symbol.asyncDispose](), undefined); t.assert.strictEqual(db.isOpen, false); }); - test.skip('prevents a database from being opened after disposal', async (t) => { + test('prevents a database from being opened after disposal', async (t) => { const db = new Database(nextDb(), { open: false }); t.assert.strictEqual(db.isOpen, false); @@ -423,7 +433,7 @@ suite('Database.prototype[Symbol.asyncDispose]()', () => { }); }); -suite.skip('Database.prototype.prepare()', () => { +suite.skip('Database.prototype.prepare()', { timeout: 1000 }, () => { test('returns a prepared statement', (t) => { const db = new Database(nextDb()); t.after(async () => { await db.close(); }); @@ -455,8 +465,8 @@ suite.skip('Database.prototype.prepare()', () => { }); }); -suite.skip('Database.prototype.exec()', () => { - test('executes SQL', async (t) => { +suite('Database.prototype.exec()', { timeout: 1000 }, () => { + test.skip('executes SQL', async (t) => { const db = new Database(nextDb()); t.after(async () => { await db.close(); }); const result = await db.exec(` @@ -487,15 +497,6 @@ suite.skip('Database.prototype.exec()', () => { }); }); - test('throws if the URL does not have the file: scheme', (t) => { - t.assert.throws(() => { - new Database(new URL('http://example.com')); - }, { - code: 'ERR_INVALID_URL_SCHEME', - message: 'The URL must be of scheme file:', - }); - }); - test('throws if database is not open', (t) => { const db = new Database(nextDb(), { open: false }); @@ -520,7 +521,7 @@ suite.skip('Database.prototype.exec()', () => { }); }); -suite.skip('Database.prototype.isTransaction', () => { +suite.skip('Database.prototype.isTransaction', { timeout: 1000 }, () => { test('correctly detects a committed transaction', async (t) => { const db = new Database(':memory:'); @@ -557,7 +558,7 @@ suite.skip('Database.prototype.isTransaction', () => { }); }); -suite.skip('Database.prototype.location()', () => { +suite.skip('Database.prototype.location()', { timeout: 1000 }, () => { test('throws if database is not open', (t) => { const db = new Database(nextDb(), { open: false }); @@ -609,8 +610,8 @@ suite.skip('Database.prototype.location()', () => { }); }); -suite.skip('Async operation ordering', () => { - test('executes operations sequentially per database', async (t) => { +suite('Async operation ordering', { timeout: 1000 }, () => { + test.skip('executes operations sequentially per database', async (t) => { const db = new Database(':memory:'); t.after(async () => { await db.close(); }); await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, seq INTEGER)'); @@ -634,7 +635,7 @@ suite.skip('Async operation ordering', () => { } }); - test('different connections can execute in parallel', async (t) => { + test('different connections can execute in parallel', { timeout: 5000 }, async (t) => { const db1 = new Database(':memory:'); const db2 = new Database(':memory:'); t.after(() => { db1.close(); db2.close(); }); From 4c068d5ddc7217025d19e625ae10fae963df478e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 26 Feb 2026 15:30:37 +0000 Subject: [PATCH 09/15] sqlite: add async Statement creation/disposal support --- src/env_properties.h | 1 + src/node_sqlite.cc | 242 ++++++++++++++++++- src/node_sqlite.h | 39 ++- test/parallel/test-sqlite-database-async.mjs | 6 +- test/parallel/test-sqlite-statement-async.js | 27 +-- 5 files changed, 284 insertions(+), 31 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index 75346b18d2b7df..3d95c15762f6f9 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -438,6 +438,7 @@ V(sqlite_column_template, v8::DictionaryTemplate) \ V(sqlite_limits_template, v8::ObjectTemplate) \ V(sqlite_run_result_template, v8::DictionaryTemplate) \ + V(sqlite_statement_constructor_template, v8::FunctionTemplate) \ V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \ V(sqlite_statement_sync_iterator_constructor_template, v8::FunctionTemplate) \ V(sqlite_session_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index f4f502dc715615..a95ada77a39161 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3856,8 +3856,42 @@ class alignas(64) OperationResult { resolver->Resolve(context, Undefined(isolate)).Check(); } }; + class PreparedStatement { + public: + explicit PreparedStatement(BaseObjectPtr db, sqlite3_stmt* stmt) + : db_(std::move(db)), stmt_(stmt) {} + + void Connect(Isolate* isolate, + Local context, + Promise::Resolver* resolver) const { + auto* stmt = stmt_; + if (!db_->IsOpen()) { + // Database is closing, therefore directly create a disposed Statement. + (void)sqlite3_finalize(stmt); + stmt = nullptr; + } + auto stmt_obj = + Statement::Create(Environment::GetCurrent(context), db_, stmt); + if (stmt_obj) { + resolver->Resolve(context, stmt_obj->object()).Check(); + } else { + Local error_message = + String::NewFromUtf8(isolate, + "Failed to create Statement object", + NewStringType::kNormal) + .ToLocalChecked(); + Local error = + Exception::Error(error_message)->ToObject(context).ToLocalChecked(); + resolver->Reject(context, error).Check(); + } + } - using variant_type = std::variant; + private: + BaseObjectPtr db_; + sqlite3_stmt* stmt_; + }; + + using variant_type = std::variant; public: static OperationResult RejectErrorCode(OperationBase* origin, @@ -3871,6 +3905,11 @@ class alignas(64) OperationResult { static OperationResult ResolveVoid(OperationBase* origin) { return OperationResult{origin, Void{}}; } + static OperationResult ResolvePreparedStatement(OperationBase* origin, + BaseObjectPtr db, + sqlite3_stmt* stmt) { + return OperationResult{origin, PreparedStatement{std::move(db), stmt}}; + } template requires std::constructible_from @@ -3892,6 +3931,47 @@ class alignas(64) OperationResult { variant_type result_; }; +class PrepareStatementOperation : private OperationBase { + public: + PrepareStatementOperation(Global&& resolver, + BaseObjectPtr&& db, + std::pmr::string&& sql) + : OperationBase(std::move(resolver)), + db_(std::move(db)), + sql_(std::move(sql)) {} + + OperationResult operator()(sqlite3* connection) { + sqlite3_stmt* stmt = nullptr; + int error_code = + sqlite3_prepare_v2(connection, sql_.c_str(), -1, &stmt, nullptr); + return error_code == SQLITE_OK + ? OperationResult::ResolvePreparedStatement( + this, std::move(db_), stmt) + : OperationResult::RejectLastError(this, connection); + } + + private: + BaseObjectPtr db_; + std::pmr::string sql_; +}; + +class FinalizeStatementOperation : private OperationBase { + public: + FinalizeStatementOperation(Global&& resolver, + sqlite3_stmt* stmt) + : OperationBase(std::move(resolver)), stmt_(stmt) {} + + OperationResult operator()(sqlite3* connection) { + int error_code = sqlite3_finalize(stmt_); + CHECK_NE(error_code, SQLITE_MISUSE); + stmt_ = nullptr; + return OperationResult::ResolveVoid(this); + } + + private: + sqlite3_stmt* stmt_; +}; + class ExecOperation : private OperationBase { public: ExecOperation(Global&& resolver, std::pmr::string&& sql) @@ -3927,7 +4007,21 @@ class CloseOperation : private OperationBase { } }; -using Operation = std::variant; +using Operation = std::variant; + +template +struct is_contained_in_variant; +template +struct is_contained_in_variant> { + static constexpr bool value{(std::is_same_v || ...)}; +}; + +template +inline constexpr bool is_operation_type = + is_contained_in_variant::value; enum class QueuePushResult { kQueueFull = -1, @@ -3955,7 +4049,8 @@ class DatabaseOperationQueue { : QueuePushResult::kSuccess; } template - requires std::constructible_from && + std::constructible_from, Global&&, Args&&...> @@ -4214,6 +4309,7 @@ v8::Local CreateDatabaseConstructorTemplate( SetProtoMethod(isolate, tmpl, "close", Database::Close); SetProtoAsyncDispose(isolate, tmpl, Database::AsyncDispose); + SetProtoMethod(isolate, tmpl, "prepare", Database::Prepare); SetProtoMethod(isolate, tmpl, "exec", Database::Exec); Local sqlite_type_key = FIXED_ONE_BYTE_STRING(isolate, "sqlite-type"); @@ -4299,13 +4395,16 @@ Local Database::AsyncDisposeImpl() { // We can't use Schedule here, because Schedule queues a MicroTask which // would try to access the Database object after destruction if this is // called from the destructor. - if (next_batch_ == nullptr) { - next_batch_ = std::make_unique( - 1U, std::pmr::get_default_resource()); - } + // Furthermore we always need to schedule the CloseOperation in a separate + // batch, to ensure that it runs after all previously scheduled operations, + // because e.g. PrepareStatementOperations need to connect their results + // first. + ProcessNextBatch(); + next_batch_ = std::make_unique( + 1U, std::pmr::get_default_resource()); CHECK_NE(next_batch_->PushEmplace(isolate, resolver), QueuePushResult::kQueueFull); - executor_->ScheduleBatch(std::move(next_batch_)); + ProcessNextBatch(); executor_.release()->Dispose(); connection_ = nullptr; @@ -4354,9 +4453,43 @@ void Database::AsyncDispose(const v8::FunctionCallbackInfo& args) { return; } + // We don't need to dispose statements during destruction, because all + // Statement instances keep a BaseObjectPtr to the Database and therefore + // can't outlive the Database. Therefore this shouldn't be executed as part of + // db->AsyncDisposeImpl(). + // Calling stmt->Dispose modifies the statements_ set, therefore make a copy. + for (Statement* stmt : std::vector(db->statements_.begin(), + db->statements_.end())) { + stmt->Dispose(); + } + args.GetReturnValue().Set(db->AsyncDisposeImpl()); } +void Database::Prepare(const v8::FunctionCallbackInfo& args) { + Database* db; + ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); + Environment* env = Environment::GetCurrent(args); + // TODO(BurningEnlightenment): these should be rejections + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + + if (!args[0]->IsString()) { + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"sql\" argument must be a string."); + return; + } + Utf8Value sql(env->isolate(), args[0].As()); + args.GetReturnValue().Set(db->Schedule( + BaseObjectPtr(db), std::pmr::string(*sql, sql.length()))); +} + +void Database::TrackStatement(Statement* statement) { + statements_.insert(statement); +} +void Database::UntrackStatement(Statement* statement) { + statements_.erase(statement); +} + void Database::Exec(const v8::FunctionCallbackInfo& args) { Database* db; // TODO(BurningEnlightenment): these should be rejections @@ -4374,6 +4507,94 @@ void Database::Exec(const v8::FunctionCallbackInfo& args) { db->Schedule(std::pmr::string(*sql, sql.length()))); } +Statement::Statement(Environment* env, + v8::Local object, + BaseObjectPtr db, + sqlite3_stmt* stmt) + : BaseObject(env, object), db_(std::move(db)), statement_(stmt) { + if (stmt == nullptr) { + db_ = nullptr; + } else { + CHECK_NOT_NULL(db_); + db_->TrackStatement(this); + } +} + +Statement::~Statement() { + if (!IsDisposed()) { + // Our operations keep a BaseObjectPtr to this Statement, so we can be sure + // that no operations are running or will run after this point. The only + // exception to this is the FinalizeStatementOperation, but it can only be + // queued by Statement::Dispose. + sqlite3_finalize(statement_); + db_->UntrackStatement(this); + } +} + +void Statement::MemoryInfo(MemoryTracker* tracker) const {} + +Local Statement::GetConstructorTemplate(Environment* env) { + Local tmpl = env->sqlite_statement_constructor_template(); + if (tmpl.IsEmpty()) { + Isolate* isolate = env->isolate(); + tmpl = NewFunctionTemplate(isolate, IllegalConstructor); + tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "Statement")); + tmpl->InstanceTemplate()->SetInternalFieldCount( + Statement::kInternalFieldCount); + + SetProtoDispose(isolate, tmpl, Statement::Dispose); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "isDisposed"), + Statement::IsDisposedGetter); + + env->set_sqlite_statement_constructor_template(tmpl); + } + return tmpl; +} +BaseObjectPtr Statement::Create(Environment* env, + BaseObjectPtr db, + sqlite3_stmt* stmt) { + Local obj; + if (!GetConstructorTemplate(env) + ->InstanceTemplate() + ->NewInstance(env->context()) + .ToLocal(&obj)) { + return nullptr; + } + + return MakeBaseObject(env, obj, std::move(db), stmt); +} + +void Statement::Dispose(const FunctionCallbackInfo& args) { + Statement* stmt; + ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); + stmt->Dispose(); + args.GetReturnValue().SetUndefined(); +} + +void Statement::Dispose() { + if (IsDisposed()) { + return; + } + // Finalizing is a no-fail operation, so we don't need to check the result or + // return a promise. + (void)db_->Schedule( + std::exchange(statement_, nullptr)); + std::exchange(db_, nullptr)->UntrackStatement(this); +} + +void Statement::IsDisposedGetter(const FunctionCallbackInfo& args) { + Statement* stmt; + ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); + Environment* env = Environment::GetCurrent(args); + args.GetReturnValue().Set(Boolean::New(env->isolate(), stmt->IsDisposed())); +} + +bool Statement::IsDisposed() const { + return statement_ == nullptr; +} + void DefineConstants(Local target) { NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_OMIT); NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_REPLACE); @@ -4450,9 +4671,8 @@ static void Initialize(Local target, context, target, "Session", Session::GetConstructorTemplate(env)); SetConstructorFunction( context, target, "Database", CreateDatabaseConstructorTemplate(env)); - target - ->Set(context, FIXED_ONE_BYTE_STRING(isolate, "Statement"), Null(isolate)) - .Check(); + SetConstructorFunction( + context, target, "Statement", Statement::GetConstructorTemplate(env)); target->Set(context, env->constants_string(), constants).Check(); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index cfcae7c42745ca..90f76adc7689aa 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -450,6 +450,7 @@ class DatabaseSyncLimits : public BaseObject { class DatabaseOperationExecutor; class DatabaseOperationQueue; +class Statement; class Database final : public DatabaseCommon { public: @@ -467,8 +468,15 @@ class Database final : public DatabaseCommon { static void New(const v8::FunctionCallbackInfo& args); static void Close(const v8::FunctionCallbackInfo& args); static void AsyncDispose(const v8::FunctionCallbackInfo& args); + static void Prepare(const v8::FunctionCallbackInfo& args); static void Exec(const v8::FunctionCallbackInfo& args); + template + [[nodiscard]] v8::Local Schedule(Args&&... args); + + void TrackStatement(Statement* statement); + void UntrackStatement(Statement* statement); + SET_MEMORY_INFO_NAME(Database) SET_SELF_SIZE(Database) @@ -482,8 +490,6 @@ class Database final : public DatabaseCommon { void PrepareNextBatch(); void ProcessNextBatch(); template - [[nodiscard]] v8::Local Schedule(Args&&... args); - template void Schedule(v8::Isolate* isolate, v8::Local resolver, Args&&... args); @@ -491,9 +497,36 @@ class Database final : public DatabaseCommon { std::unique_ptr executor_; std::unique_ptr next_batch_; + std::unordered_set statements_; + static constexpr int kDefaultBatchSize = 31; +}; - friend class StatementExecutionHelper; +class Statement final : public BaseObject { + public: + Statement(Environment* env, + v8::Local object, + BaseObjectPtr db, + sqlite3_stmt* stmt); + void MemoryInfo(MemoryTracker* tracker) const override; + static v8::Local GetConstructorTemplate( + Environment* env); + static BaseObjectPtr Create(Environment* env, + BaseObjectPtr db, + sqlite3_stmt* stmt); + static void Dispose(const v8::FunctionCallbackInfo& args); + void Dispose(); + static void IsDisposedGetter(const v8::FunctionCallbackInfo& args); + bool IsDisposed() const; + + SET_MEMORY_INFO_NAME(Statement) + SET_SELF_SIZE(Statement) + + private: + ~Statement() override; + + BaseObjectPtr db_; + sqlite3_stmt* statement_; }; } // namespace sqlite diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index 5fd874f506dc51..6c28a9ec83e584 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -433,11 +433,11 @@ suite('Database.prototype[Symbol.asyncDispose]()', { timeout: 1000 }, () => { }); }); -suite.skip('Database.prototype.prepare()', { timeout: 1000 }, () => { - test('returns a prepared statement', (t) => { +suite('Database.prototype.prepare()', { timeout: 1000 }, () => { + test('returns a prepared statement', async (t) => { const db = new Database(nextDb()); t.after(async () => { await db.close(); }); - const stmt = db.prepare('CREATE TABLE webstorage(key TEXT)'); + using stmt = await db.prepare('CREATE TABLE webstorage(key TEXT)'); t.assert.ok(stmt instanceof Statement); }); diff --git a/test/parallel/test-sqlite-statement-async.js b/test/parallel/test-sqlite-statement-async.js index 517c07cbf60a3a..16bbafac9eb8dc 100644 --- a/test/parallel/test-sqlite-statement-async.js +++ b/test/parallel/test-sqlite-statement-async.js @@ -1,7 +1,6 @@ // Flags: --expose-gc 'use strict'; -const { skip, skipIfSQLiteMissing } = require('../common'); -skip(); // TODO: Re-enable when async Statement support is added. +const { skipIfSQLiteMissing } = require('../common'); skipIfSQLiteMissing(); const tmpdir = require('../common/tmpdir'); const { join } = require('node:path'); @@ -26,7 +25,7 @@ suite('Statement() constructor', () => { }); }); -suite('Statement.prototype.get()', () => { +suite.skip('Statement.prototype.get()', () => { test('executes a query and returns undefined on no results', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -56,7 +55,7 @@ suite('Statement.prototype.get()', () => { }); }); -suite('Statement.prototype.all()', () => { +suite.skip('Statement.prototype.all()', () => { test('executes a query and returns an empty array on no results', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -86,7 +85,7 @@ suite('Statement.prototype.all()', () => { }); }); -suite('Statement.prototype.iterate()', () => { +suite.skip('Statement.prototype.iterate()', () => { test('executes a query and returns an empty iterator on no results', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -174,7 +173,7 @@ suite('Statement.prototype.iterate()', () => { }); }); -suite('Statement.prototype.run()', () => { +suite.skip('Statement.prototype.run()', () => { test('executes a query and returns change metadata', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -273,7 +272,7 @@ suite('Statement.prototype.run()', () => { }); }); -suite('Statement.prototype.sourceSQL', () => { +suite.skip('Statement.prototype.sourceSQL', () => { test('equals input SQL', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -287,7 +286,7 @@ suite('Statement.prototype.sourceSQL', () => { }); }); -suite('Statement.prototype.expandedSQL', async () => { +suite.skip('Statement.prototype.expandedSQL', async () => { test('equals expanded SQL', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -306,7 +305,7 @@ suite('Statement.prototype.expandedSQL', async () => { }); }); -suite('Statement.prototype.setReadBigInts()', () => { +suite.skip('Statement.prototype.setReadBigInts()', () => { test('BigInts support can be toggled', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -375,7 +374,7 @@ suite('Statement.prototype.setReadBigInts()', () => { }); }); -suite('Statement.prototype.setReturnArrays()', () => { +suite.skip('Statement.prototype.setReturnArrays()', () => { test('throws when input is not a boolean', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -393,7 +392,7 @@ suite('Statement.prototype.setReturnArrays()', () => { }); }); -suite('Statement.prototype.get() with array output', () => { +suite.skip('Statement.prototype.get() with array output', () => { test('returns array row when setReturnArrays is true', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -432,7 +431,7 @@ suite('Statement.prototype.get() with array output', () => { }); }); -suite('Statement.prototype.all() with array output', () => { +suite.skip('Statement.prototype.all() with array output', () => { test('returns array rows when setReturnArrays is true', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -498,7 +497,7 @@ suite('Statement.prototype.all() with array output', () => { }); }); -suite('Statement.prototype.iterate() with array output', () => { +suite.skip('Statement.prototype.iterate() with array output', () => { test('iterates array rows when setReturnArrays is true', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); @@ -567,7 +566,7 @@ suite('Statement.prototype.iterate() with array output', () => { }); }); -suite('Statement.prototype.setAllowBareNamedParameters()', () => { +suite.skip('Statement.prototype.setAllowBareNamedParameters()', () => { test('bare named parameter support can be toggled', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); From 855837bdfe2562851e43ab9703e32f5ae2ea8417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 26 Feb 2026 21:05:39 +0000 Subject: [PATCH 10/15] sqlite: implement basic async value passing and get --- src/node_sqlite.cc | 383 ++++++++++++++++++- src/node_sqlite.h | 12 + test/parallel/test-sqlite-database-async.mjs | 4 +- test/parallel/test-sqlite-statement-async.js | 46 ++- 4 files changed, 424 insertions(+), 21 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index a95ada77a39161..182fbcbe537c8a 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3765,6 +3765,306 @@ void Session::Delete() { } namespace { +namespace transfer { +struct null {}; +struct boolean { + bool value; +}; +struct integer { + sqlite3_int64 value; +}; +struct real { + double value; +}; +struct text { + std::pmr::string value; + + text() = default; + explicit text(std::pmr::string&& str) : value(std::move(str)) {} + explicit text(std::string_view str) : value(str.data(), str.size()) {} + text(const char* str, size_t len) : value(str, len) {} +}; +struct blob { + std::pmr::vector value; + + blob() = default; + explicit blob(std::pmr::vector&& vec) : value(std::move(vec)) {} + explicit blob(std::span span) + : value(span.begin(), span.end()) {} + blob(const uint8_t* data, size_t len) : value(data, data + len) {} +}; +using literal = std::variant; +using value = std::variant, + std::pmr::unordered_map>; + +struct bind_literal { + sqlite3_stmt* stmt; + int index; + + int operator()(null) const { return sqlite3_bind_null(stmt, index); } + int operator()(boolean b) const { + return sqlite3_bind_int(stmt, index, b.value ? 1 : 0); + } + int operator()(integer i) const { + return sqlite3_bind_int64(stmt, index, i.value); + } + int operator()(real r) const { + return sqlite3_bind_double(stmt, index, r.value); + } + int operator()(const text& t) const { + return sqlite3_bind_text64(stmt, + index, + t.value.data(), + static_cast(t.value.size()), + SQLITE_STATIC, + SQLITE_UTF8); + } + int operator()(const blob& b) const { + return sqlite3_bind_blob64(stmt, + index, + b.value.data(), + static_cast(b.value.size()), + SQLITE_STATIC); + } + int operator()(const transfer::literal& literal) const { + return std::visit(*this, literal); + } +}; + +struct bind_value { + sqlite3_stmt* stmt; + + int operator()(const std::pmr::vector& value) const { + if (!std::in_range(value.size())) [[unlikely]] { + return SQLITE_RANGE; + } + bind_literal binder{stmt, static_cast(value.size())}; + for (; binder.index > 0; --binder.index) { + if (int r = binder(value[binder.index - 1]); r != SQLITE_OK) { + return r; + } + } + return SQLITE_OK; + } + int operator()( + const std::pmr::unordered_map& value) + const { + bind_literal binder{stmt, 0}; + for (const auto& [name, value] : value) { + binder.index = sqlite3_bind_parameter_index(stmt, name.c_str()); + if (binder.index == 0) { + // No such named parameter, ignore this key. + continue; + } + if (int r = binder(value); r != SQLITE_OK) { + return r; + } + } + return SQLITE_OK; + } + int operator()(const transfer::value& value) const { + return std::visit(*this, value); + } +}; + +literal FromColumn(sqlite3* db, sqlite3_stmt* stmt, int col_index) { + using std::in_place_type; + int type = sqlite3_column_type(stmt, col_index); + switch (type) { + case SQLITE_NULL: + return literal{in_place_type}; + case SQLITE_INTEGER: + return literal{in_place_type, + sqlite3_column_int64(stmt, col_index)}; + case SQLITE_FLOAT: + return literal{in_place_type, + sqlite3_column_double(stmt, col_index)}; + case SQLITE_TEXT: { + const char* text_value = + reinterpret_cast(sqlite3_column_text(stmt, col_index)); + CHECK_NOT_NULL(text_value); // Catch OOM condition. + int size = sqlite3_column_bytes(stmt, col_index); + return literal{ + in_place_type, text_value, static_cast(size)}; + } + case SQLITE_BLOB: { + const uint8_t* blob_value = reinterpret_cast( + sqlite3_column_blob(stmt, col_index)); + if (blob_value == nullptr) [[unlikely]] { + CHECK_NE(sqlite3_errcode(db), SQLITE_NOMEM); + return literal{in_place_type}; + } + int size = sqlite3_column_bytes(stmt, col_index); + return literal{ + in_place_type, blob_value, static_cast(size)}; + } + default: + UNREACHABLE("Bad SQLite value"); + } +}; + +struct to_v8_value { + Isolate* isolate; + + Local operator()(const null&) const { return Null(isolate); } + Local operator()(const boolean& b) const { + return Boolean::New(isolate, b.value); + } + Local operator()(const integer& i) const { + constexpr auto kMaxSafeInteger = (sqlite3_int64{1} << 53) - 1; // 2^53 - 1 + constexpr auto kMinSafeInteger = -kMaxSafeInteger; // -(2^53 - 1) + + if (kMinSafeInteger <= i.value && i.value <= kMaxSafeInteger) { + return Number::New(isolate, static_cast(i.value)); + } else { + return BigInt::New(isolate, i.value); + } + } + Local operator()(const real& r) const { + return Number::New(isolate, r.value); + } + Local operator()(const text& t) const { + return String::NewFromUtf8( + isolate, t.value.data(), NewStringType::kNormal, t.value.size()) + .ToLocalChecked(); + } + Local operator()(const blob& b) const { + const auto blobSize = b.value.size(); + auto buffer = ArrayBuffer::New( + isolate, blobSize, BackingStoreInitializationMode::kUninitialized); + std::memcpy(buffer->Data(), b.value.data(), blobSize); + return Uint8Array::New(buffer, 0, blobSize); + } + Local operator()(const transfer::literal& literal) const { + return std::visit(*this, literal); + } + Local operator()( + const std::pmr::vector& vec) const { + Local context = isolate->GetCurrentContext(); + Local array = Array::New(isolate, vec.size()); + for (size_t i = 0; i < vec.size(); ++i) { + Local element = (*this)(vec[i]); + // TODO(BurningEnlightenment): Proper bounds checking + array->Set(context, static_cast(i), element).Check(); + } + return array; + } + Local operator()( + const std::pmr::unordered_map& map) + const { + Local context = isolate->GetCurrentContext(); + Local obj = Object::New(isolate); + for (const auto& [key, value] : map) { + Local v8_value = (*this)(value); + // TODO(BurningEnlightenment): Crash on OOM ok? + obj->Set(context, + String::NewFromUtf8( + isolate, key.data(), NewStringType::kNormal, key.size()) + .ToLocalChecked(), + v8_value) + .Check(); + } + return obj; + } + Local operator()(const transfer::value& value) const { + return std::visit(*this, value); + } +}; + +Maybe ToLiteral(Isolate* isolate, Local value) { + using std::in_place_type; + if (value->IsNumber()) { + return v8::Just(literal{in_place_type, value.As()->Value()}); + } else if (value->IsString()) { + Utf8Value utf8_value(isolate, value.As()); + return v8::Just( + literal{in_place_type, *utf8_value, utf8_value.length()}); + } else if (value->IsNull()) { + return v8::Just(literal{in_place_type}); + } else if (value->IsBigInt()) { + bool lossless{}; + sqlite3_int64 int_value = value.As()->Int64Value(&lossless); + if (!lossless) { + THROW_ERR_INVALID_ARG_VALUE(isolate, + "BigInt value is too large to bind."); + return {}; + } + return v8::Just(literal{in_place_type, int_value}); + } else if (value->IsArrayBufferView()) { + ArrayBufferViewContents buf(value); + return v8::Just(literal{in_place_type, buf.data(), buf.length()}); + } else [[unlikely]] { + THROW_ERR_INVALID_ARG_TYPE( + isolate, "Provided value cannot be bound to SQLite parameter."); + return {}; + } +} +Maybe ToValue(Isolate* isolate, Local object) { + using std::in_place_type; + Local property_names; + if (!object->GetOwnPropertyNames(isolate->GetCurrentContext()) + .ToLocal(&property_names)) [[unlikely]] { + return {}; + } + const uint32_t length = property_names->Length(); + Local context = isolate->GetCurrentContext(); + std::pmr::unordered_map map; + map.reserve(length); + for (uint32_t i = 0; i < length; ++i) { + Local key; + if (!property_names->Get(context, i).ToLocal(&key) || !key->IsString()) + [[unlikely]] { + THROW_ERR_INVALID_ARG_TYPE( + isolate, + "Object keys must be strings to morph to SQLite parameters."); + return {}; + } + Utf8Value utf8_key(isolate, key.As()); + Local value; + if (!object->Get(context, key).ToLocal(&value)) [[unlikely]] { + return {}; + } + auto maybe_literal = ToLiteral(isolate, value); + if (maybe_literal.IsNothing()) [[unlikely]] { + return {}; + } + map.emplace(utf8_key.operator*(), std::move(maybe_literal).FromJust()); + } + return v8::Just(value{in_place_type, std::move(map)}); +} +Maybe ToValue(Isolate* isolate, Local array) { + using std::in_place_type; + const uint32_t length = array->Length(); + Local context = isolate->GetCurrentContext(); + std::pmr::vector vec; + vec.reserve(length); + for (uint32_t i = 0; i < length; ++i) { + Local value; + if (!array->Get(context, i).ToLocal(&value)) [[unlikely]] { + return {}; + } + auto maybe_literal = ToLiteral(isolate, value); + if (maybe_literal.IsNothing()) [[unlikely]] { + return {}; + } + vec.push_back(std::move(maybe_literal).FromJust()); + } + return v8::Just(value{in_place_type, std::move(vec)}); +} +Maybe ToValue(Isolate* isolate, Local value) { + if (value->IsArray()) { + return ToValue(isolate, value.As()); + } else if (value->IsObject()) { + return ToValue(isolate, value.As()); + } else [[unlikely]] { + THROW_ERR_INVALID_ARG_TYPE( + isolate, + "Value must be an object or array to morph to SQLite parameters."); + return {}; + } +} +} // namespace transfer + class OperationBase { public: Local GetResolver(Isolate* isolate) const { @@ -3890,8 +4190,22 @@ class alignas(64) OperationResult { BaseObjectPtr db_; sqlite3_stmt* stmt_; }; + class Value { + public: + explicit Value(transfer::value value) : value_(std::move(value)) {} - using variant_type = std::variant; + void Connect(Isolate* isolate, + Local context, + Promise::Resolver* resolver) const { + resolver->Resolve(context, transfer::to_v8_value(isolate)(value_)) + .Check(); + } + + private: + transfer::value value_; + }; + + using variant_type = std::variant; public: static OperationResult RejectErrorCode(OperationBase* origin, @@ -3905,6 +4219,10 @@ class alignas(64) OperationResult { static OperationResult ResolveVoid(OperationBase* origin) { return OperationResult{origin, Void{}}; } + static OperationResult ResolveValue(OperationBase* origin, + transfer::value&& value) { + return OperationResult{origin, Value{std::move(value)}}; + } static OperationResult ResolvePreparedStatement(OperationBase* origin, BaseObjectPtr db, sqlite3_stmt* stmt) { @@ -3972,6 +4290,47 @@ class FinalizeStatementOperation : private OperationBase { sqlite3_stmt* stmt_; }; +class StatementGetOperation : private OperationBase { + public: + StatementGetOperation(Global&& resolver, + sqlite3_stmt* stmt, + transfer::value&& bind_arguments) + : OperationBase(std::move(resolver)), + stmt_(stmt), + bind_arguments_(std::move(bind_arguments)) {} + + OperationResult operator()(sqlite3* connection) { + auto clear_bindings = OnScopeLeave([&] { sqlite3_clear_bindings(stmt_); }); + if (int r = transfer::bind_value(stmt_)(bind_arguments_); r != SQLITE_OK) { + return OperationResult::RejectErrorCode(this, r); + } + auto reset_statement = OnScopeLeave([&] { sqlite3_reset(stmt_); }); + int error_code = sqlite3_step(stmt_); + if (error_code == SQLITE_DONE) { + return OperationResult::ResolveVoid(this); + } + if (error_code != SQLITE_ROW) { + return OperationResult::RejectLastError(this, connection); + } + + int num_cols = sqlite3_column_count(stmt_); + if (num_cols == 0) { + return OperationResult::ResolveVoid(this); + } + + std::pmr::vector row; + row.reserve(num_cols); + for (int i = 0; i < num_cols; ++i) { + row.push_back(transfer::FromColumn(connection, stmt_, i)); + } + return OperationResult::ResolveValue(this, std::move(row)); + } + + private: + sqlite3_stmt* stmt_; + transfer::value bind_arguments_; +}; + class ExecOperation : private OperationBase { public: ExecOperation(Global&& resolver, std::pmr::string&& sql) @@ -4008,6 +4367,7 @@ class CloseOperation : private OperationBase { }; using Operation = std::variant; @@ -4512,6 +4872,7 @@ Statement::Statement(Environment* env, BaseObjectPtr db, sqlite3_stmt* stmt) : BaseObject(env, object), db_(std::move(db)), statement_(stmt) { + MakeWeak(); if (stmt == nullptr) { db_ = nullptr; } else { @@ -4547,6 +4908,7 @@ Local Statement::GetConstructorTemplate(Environment* env) { tmpl, FIXED_ONE_BYTE_STRING(isolate, "isDisposed"), Statement::IsDisposedGetter); + SetProtoMethod(isolate, tmpl, "get", Statement::Get); env->set_sqlite_statement_constructor_template(tmpl); } @@ -4595,6 +4957,25 @@ bool Statement::IsDisposed() const { return statement_ == nullptr; } +void Statement::Get(const v8::FunctionCallbackInfo& args) { + Statement* stmt; + ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); + Environment* env = Environment::GetCurrent(args); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsDisposed(), "statement is disposed"); + + transfer::value bind_arguments; + if (args.Length() > 1) { + if (!transfer::ToValue(env->isolate(), args[0].As()) + .MoveTo(&bind_arguments)) { + return; + } + } + + args.GetReturnValue().Set(stmt->db_->Schedule( + stmt->statement_, std::move(bind_arguments))); +} + void DefineConstants(Local target) { NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_OMIT); NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_REPLACE); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 90f76adc7689aa..3268720645065d 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -466,10 +466,20 @@ class Database final : public DatabaseCommon { bool allow_load_extension); void MemoryInfo(MemoryTracker* tracker) const override; static void New(const v8::FunctionCallbackInfo& args); + // static void IsTransactionGetter( + // const v8::FunctionCallbackInfo& args); static void Close(const v8::FunctionCallbackInfo& args); static void AsyncDispose(const v8::FunctionCallbackInfo& args); static void Prepare(const v8::FunctionCallbackInfo& args); static void Exec(const v8::FunctionCallbackInfo& args); + // static void CreateTagStore(const v8::FunctionCallbackInfo& + // args); static void Location(const v8::FunctionCallbackInfo& + // args); static void EnableLoadExtension( + // const v8::FunctionCallbackInfo& args); + // static void EnableDefensive(const v8::FunctionCallbackInfo& + // args); static void LoadExtension(const v8::FunctionCallbackInfo& + // args); void FinalizeStatements(); void UntrackStatement(StatementSync* + // statement); template [[nodiscard]] v8::Local Schedule(Args&&... args); @@ -519,6 +529,8 @@ class Statement final : public BaseObject { static void IsDisposedGetter(const v8::FunctionCallbackInfo& args); bool IsDisposed() const; + static void Get(const v8::FunctionCallbackInfo& args); + SET_MEMORY_INFO_NAME(Statement) SET_SELF_SIZE(Statement) diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index 6c28a9ec83e584..8447f0a570893f 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -224,7 +224,7 @@ suite('Database() constructor', { timeout: 1000 }, () => { }); }); - test.skip('allows returning arrays', async (t) => { + test('allows returning arrays', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { returnArrays: true }); t.after(async () => { await db.close(); }); @@ -235,7 +235,7 @@ suite('Database() constructor', { timeout: 1000 }, () => { `); t.assert.strictEqual(setup, undefined); - const query = db.prepare('SELECT key, val FROM data WHERE key = 1'); + using query = await db.prepare('SELECT key, val FROM data WHERE key = 1'); t.assert.deepStrictEqual(await query.get(), [1, 'one']); }); diff --git a/test/parallel/test-sqlite-statement-async.js b/test/parallel/test-sqlite-statement-async.js index 16bbafac9eb8dc..eb0928555b3a4c 100644 --- a/test/parallel/test-sqlite-statement-async.js +++ b/test/parallel/test-sqlite-statement-async.js @@ -25,32 +25,42 @@ suite('Statement() constructor', () => { }); }); -suite.skip('Statement.prototype.get()', () => { +suite('Statement.prototype.get()', () => { test('executes a query and returns undefined on no results', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); - let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); - t.assert.strictEqual(await stmt.get(), undefined); - stmt = db.prepare('SELECT * FROM storage'); - t.assert.strictEqual(await stmt.get(), undefined); + t.after(async () => { await db.close(); }); + { + using stmt = await db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.strictEqual(await stmt.get(), undefined); + } + { + using stmt = await db.prepare('SELECT * FROM storage'); + t.assert.strictEqual(await stmt.get(), undefined); + } }); - test('executes a query and returns the first result', async (t) => { + test.skip('executes a query and returns the first result', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); - let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); - t.assert.strictEqual(await stmt.get(), undefined); - stmt = db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)'); - t.assert.strictEqual(await stmt.get('key1', 'val1'), undefined); - t.assert.strictEqual(await stmt.get('key2', 'val2'), undefined); - stmt = db.prepare('SELECT * FROM storage ORDER BY key'); - t.assert.deepStrictEqual(await stmt.get(), { __proto__: null, key: 'key1', val: 'val1' }); + t.after(async () => { await db.close(); }); + { + using stmt = await db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.strictEqual(await stmt.get(), undefined); + } + { + using stmt = await db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)'); + t.assert.strictEqual(await stmt.get('key1', 'val1'), undefined); + t.assert.strictEqual(await stmt.get('key2', 'val2'), undefined); + } + { + using stmt = await db.prepare('SELECT * FROM storage ORDER BY key'); + t.assert.deepStrictEqual(await stmt.get(), { __proto__: null, key: 'key1', val: 'val1' }); + } }); - test('executes a query that returns special columns', async (t) => { + test.skip('executes a query that returns special columns', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); - const stmt = db.prepare('SELECT 1 as __proto__, 2 as constructor, 3 as toString'); + t.after(async () => { await db.close(); }); + using stmt = await db.prepare('SELECT 1 as __proto__, 2 as constructor, 3 as toString'); t.assert.deepStrictEqual(await stmt.get(), { __proto__: null, ['__proto__']: 1, constructor: 2, toString: 3 }); }); }); From 1eceaf7b856e497188ebf0dcb1cf8ed4c00f7b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Thu, 26 Feb 2026 23:48:28 +0000 Subject: [PATCH 11/15] sqlite: remove function-local using-statements The linter doesn't seem to like these. Is it a linter bug? --- src/node_sqlite.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 182fbcbe537c8a..244331edc48eb6 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -23,6 +23,7 @@ namespace node { namespace sqlite { +using std::in_place_type; using v8::Array; using v8::ArrayBuffer; using v8::BackingStoreInitializationMode; @@ -3868,7 +3869,6 @@ struct bind_value { }; literal FromColumn(sqlite3* db, sqlite3_stmt* stmt, int col_index) { - using std::in_place_type; int type = sqlite3_column_type(stmt, col_index); switch (type) { case SQLITE_NULL: @@ -3972,7 +3972,6 @@ struct to_v8_value { }; Maybe ToLiteral(Isolate* isolate, Local value) { - using std::in_place_type; if (value->IsNumber()) { return v8::Just(literal{in_place_type, value.As()->Value()}); } else if (value->IsString()) { @@ -4000,7 +3999,6 @@ Maybe ToLiteral(Isolate* isolate, Local value) { } } Maybe ToValue(Isolate* isolate, Local object) { - using std::in_place_type; Local property_names; if (!object->GetOwnPropertyNames(isolate->GetCurrentContext()) .ToLocal(&property_names)) [[unlikely]] { @@ -4033,7 +4031,6 @@ Maybe ToValue(Isolate* isolate, Local object) { return v8::Just(value{in_place_type, std::move(map)}); } Maybe ToValue(Isolate* isolate, Local array) { - using std::in_place_type; const uint32_t length = array->Length(); Local context = isolate->GetCurrentContext(); std::pmr::vector vec; From 226cf2b40a94ff2b194c5aa500ab40f96a8c905c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Mon, 2 Mar 2026 21:48:42 +0000 Subject: [PATCH 12/15] sqlite: implement `Statement.prototype.all()` --- src/node_sqlite.cc | 91 +++++++++- src/node_sqlite.h | 1 + test/parallel/test-sqlite-database-async.mjs | 6 +- test/parallel/test-sqlite-statement-async.js | 170 ++++--------------- 4 files changed, 126 insertions(+), 142 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 244331edc48eb6..1e377d402390c4 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -4201,8 +4201,31 @@ class alignas(64) OperationResult { private: transfer::value value_; }; + class Values { + public: + explicit Values(std::pmr::vector values) + : values_(std::move(values)) {} + + void Connect(Isolate* isolate, + Local context, + Promise::Resolver* resolver) const { + CHECK_LE(values_.size(), + static_cast(std::numeric_limits::max())); + int size = static_cast(values_.size()); + Local array = Array::New(isolate, size); + for (int i = 0; i < size; ++i) { + auto element = transfer::to_v8_value(isolate)(values_[i]); + array->Set(context, static_cast(i), element).Check(); + } + resolver->Resolve(context, array).Check(); + } - using variant_type = std::variant; + private: + std::pmr::vector values_; + }; + + using variant_type = + std::variant; public: static OperationResult RejectErrorCode(OperationBase* origin, @@ -4220,6 +4243,10 @@ class alignas(64) OperationResult { transfer::value&& value) { return OperationResult{origin, Value{std::move(value)}}; } + static OperationResult ResolveValues( + OperationBase* origin, std::pmr::vector&& values) { + return OperationResult{origin, Values{std::move(values)}}; + } static OperationResult ResolvePreparedStatement(OperationBase* origin, BaseObjectPtr db, sqlite3_stmt* stmt) { @@ -4328,6 +4355,47 @@ class StatementGetOperation : private OperationBase { transfer::value bind_arguments_; }; +class StatementAllOperation : private OperationBase { + public: + StatementAllOperation(Global&& resolver, + sqlite3_stmt* stmt, + transfer::value&& bind_arguments) + : OperationBase(std::move(resolver)), + stmt_(stmt), + bind_arguments_(std::move(bind_arguments)) {} + + OperationResult operator()(sqlite3* connection) { + auto clear_bindings = OnScopeLeave([&] { sqlite3_clear_bindings(stmt_); }); + if (int r = transfer::bind_value(stmt_)(bind_arguments_); r != SQLITE_OK) { + return OperationResult::RejectErrorCode(this, r); + } + auto reset_statement = OnScopeLeave([&] { sqlite3_reset(stmt_); }); + + std::pmr::vector rows; + int r; + int num_cols = sqlite3_column_count(stmt_); + for (r = sqlite3_step(stmt_); r == SQLITE_ROW; r = sqlite3_step(stmt_)) { + if (num_cols == 0) { + rows.emplace_back(in_place_type>); + continue; + } + + std::pmr::vector row; + row.reserve(num_cols); + for (int i = 0; i < num_cols; ++i) { + row.push_back(transfer::FromColumn(connection, stmt_, i)); + } + rows.emplace_back(in_place_type>, + std::move(row)); + } + return OperationResult::ResolveValues(this, std::move(rows)); + } + + private: + sqlite3_stmt* stmt_; + transfer::value bind_arguments_; +}; + class ExecOperation : private OperationBase { public: ExecOperation(Global&& resolver, std::pmr::string&& sql) @@ -4365,6 +4433,7 @@ class CloseOperation : private OperationBase { using Operation = std::variant; @@ -4906,6 +4975,7 @@ Local Statement::GetConstructorTemplate(Environment* env) { FIXED_ONE_BYTE_STRING(isolate, "isDisposed"), Statement::IsDisposedGetter); SetProtoMethod(isolate, tmpl, "get", Statement::Get); + SetProtoMethod(isolate, tmpl, "all", Statement::All); env->set_sqlite_statement_constructor_template(tmpl); } @@ -4973,6 +5043,25 @@ void Statement::Get(const v8::FunctionCallbackInfo& args) { stmt->statement_, std::move(bind_arguments))); } +void Statement::All(const v8::FunctionCallbackInfo& args) { + Statement* stmt; + ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); + Environment* env = Environment::GetCurrent(args); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsDisposed(), "statement is disposed"); + + transfer::value bind_arguments; + if (args.Length() > 1) { + if (!transfer::ToValue(env->isolate(), args[0].As()) + .MoveTo(&bind_arguments)) { + return; + } + } + + args.GetReturnValue().Set(stmt->db_->Schedule( + stmt->statement_, std::move(bind_arguments))); +} + void DefineConstants(Local target) { NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_OMIT); NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_REPLACE); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 3268720645065d..73abf9a0b7b76f 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -530,6 +530,7 @@ class Statement final : public BaseObject { bool IsDisposed() const; static void Get(const v8::FunctionCallbackInfo& args); + static void All(const v8::FunctionCallbackInfo& args); SET_MEMORY_INFO_NAME(Statement) SET_SELF_SIZE(Statement) diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index 8447f0a570893f..772414415ef106 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -478,7 +478,7 @@ suite('Database.prototype.exec()', { timeout: 1000 }, () => { INSERT INTO data (key, val) VALUES (8, 9); `); t.assert.strictEqual(result, undefined); - const stmt = db.prepare('SELECT * FROM data ORDER BY key'); + using stmt = await db.prepare('SELECT * FROM data ORDER BY key'); t.assert.deepStrictEqual(await stmt.all(), [ { __proto__: null, key: 1, val: 2 }, { __proto__: null, key: 8, val: 9 }, @@ -625,7 +625,7 @@ suite('Async operation ordering', { timeout: 1000 }, () => { await Promise.all(ops); // Check they were inserted in order (sequential execution) - const stmt = db.prepare('SELECT id, seq FROM test ORDER BY id'); + using stmt = await db.prepare('SELECT id, seq FROM test ORDER BY id'); const rows = await stmt.all(); // Verify sequential: id should match seq + 1 (autoincrement starts at 1) @@ -638,7 +638,7 @@ suite('Async operation ordering', { timeout: 1000 }, () => { test('different connections can execute in parallel', { timeout: 5000 }, async (t) => { const db1 = new Database(':memory:'); const db2 = new Database(':memory:'); - t.after(() => { db1.close(); db2.close(); }); + t.after(async () => { await Promise.all([db1.close(), db2.close()]); }); const times = {}; const now = () => process.hrtime.bigint(); const LONG_QUERY = ` diff --git a/test/parallel/test-sqlite-statement-async.js b/test/parallel/test-sqlite-statement-async.js index eb0928555b3a4c..9b8e9aad39cad4 100644 --- a/test/parallel/test-sqlite-statement-async.js +++ b/test/parallel/test-sqlite-statement-async.js @@ -65,33 +65,39 @@ suite('Statement.prototype.get()', () => { }); }); -suite.skip('Statement.prototype.all()', () => { +suite('Statement.prototype.all()', () => { test('executes a query and returns an empty array on no results', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); - const stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.after(async () => { await db.close(); }); + using stmt = await db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); t.assert.deepStrictEqual(await stmt.all(), []); }); - test('executes a query and returns all results', async (t) => { + test.skip('executes a query and returns all results', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); - let stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); - t.assert.deepStrictEqual(await stmt.run(), { changes: 0, lastInsertRowid: 0 }); - stmt = db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)'); - t.assert.deepStrictEqual( - await stmt.run('key1', 'val1'), - { changes: 1, lastInsertRowid: 1 }, - ); - t.assert.deepStrictEqual( - await stmt.run('key2', 'val2'), - { changes: 1, lastInsertRowid: 2 }, - ); - stmt = db.prepare('SELECT * FROM storage ORDER BY key'); - t.assert.deepStrictEqual(await stmt.all(), [ - { __proto__: null, key: 'key1', val: 'val1' }, - { __proto__: null, key: 'key2', val: 'val2' }, - ]); + t.after(async () => { await db.close(); }); + { + using stmt = await db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); + t.assert.deepStrictEqual(await stmt.run(), { changes: 0, lastInsertRowid: 0 }); + } + { + using stmt = await db.prepare('INSERT INTO storage (key, val) VALUES (?, ?)'); + t.assert.deepStrictEqual( + await stmt.run('key1', 'val1'), + { changes: 1, lastInsertRowid: 1 }, + ); + t.assert.deepStrictEqual( + await stmt.run('key2', 'val2'), + { changes: 1, lastInsertRowid: 2 }, + ); + } + { + using stmt = await db.prepare('SELECT * FROM storage ORDER BY key'); + t.assert.deepStrictEqual(await stmt.all(), [ + { __proto__: null, key: 'key1', val: 'val1' }, + { __proto__: null, key: 'key2', val: 'val2' }, + ]); + } }); }); @@ -441,8 +447,8 @@ suite.skip('Statement.prototype.get() with array output', () => { }); }); -suite.skip('Statement.prototype.all() with array output', () => { - test('returns array rows when setReturnArrays is true', async (t) => { +suite('Statement.prototype.all() with array output', () => { + test.skip('returns array rows when setReturnArrays is true', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); const setup = await db.exec(` @@ -498,124 +504,12 @@ suite.skip('Statement.prototype.all() with array output', () => { `); t.assert.strictEqual(setup, undefined); - const query = db.prepare('SELECT * FROM wide_table'); - query.setReturnArrays(true); + using query = await db.prepare('SELECT * FROM wide_table', { + returnArrays: true, + }); const results = await query.all(); t.assert.strictEqual(results.length, 1); t.assert.deepStrictEqual(results[0], expected); }); }); - -suite.skip('Statement.prototype.iterate() with array output', () => { - test('iterates array rows when setReturnArrays is true', async (t) => { - const db = new Database(nextDb()); - t.after(() => { db.close(); }); - const setup = await db.exec(` - CREATE TABLE data(key INTEGER PRIMARY KEY, val TEXT) STRICT; - INSERT INTO data (key, val) VALUES (1, 'one'); - INSERT INTO data (key, val) VALUES (2, 'two'); - `); - t.assert.strictEqual(setup, undefined); - - const query = db.prepare('SELECT key, val FROM data ORDER BY key'); - - // Test with objects first - const objectRows = []; - for (const row of query.iterate()) { - objectRows.push(row); - } - t.assert.deepStrictEqual(objectRows, [ - { __proto__: null, key: 1, val: 'one' }, - { __proto__: null, key: 2, val: 'two' }, - ]); - - // Test with arrays - query.setReturnArrays(true); - const arrayRows = []; - for (const row of query.iterate()) { - arrayRows.push(row); - } - t.assert.deepStrictEqual(arrayRows, [ - [1, 'one'], - [2, 'two'], - ]); - - // Test toArray() method - t.assert.deepStrictEqual(query.iterate().toArray(), [ - [1, 'one'], - [2, 'two'], - ]); - }); - - test('iterator can be exited early with array rows', async (t) => { - const db = new Database(':memory:'); - await db.exec(` - CREATE TABLE test(key TEXT, val TEXT); - INSERT INTO test (key, val) VALUES ('key1', 'val1'); - INSERT INTO test (key, val) VALUES ('key2', 'val2'); - `); - const stmt = db.prepare('SELECT key, val FROM test'); - stmt.setReturnArrays(true); - - const iterator = stmt.iterate(); - const results = []; - - for (const item of iterator) { - results.push(item); - break; - } - - t.assert.deepStrictEqual(results, [ - ['key1', 'val1'], - ]); - t.assert.deepStrictEqual( - iterator.next(), - { __proto__: null, done: true, value: null }, - ); - }); -}); - -suite.skip('Statement.prototype.setAllowBareNamedParameters()', () => { - test('bare named parameter support can be toggled', async (t) => { - const db = new Database(nextDb()); - t.after(() => { db.close(); }); - const setup = await db.exec( - 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' - ); - t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); - t.assert.deepStrictEqual( - await stmt.run({ k: 1, v: 2 }), - { changes: 1, lastInsertRowid: 1 }, - ); - t.assert.strictEqual(stmt.setAllowBareNamedParameters(false), undefined); - t.assert.throws(() => { - stmt.run({ k: 2, v: 4 }); - }, { - code: 'ERR_INVALID_STATE', - message: /Unknown named parameter 'k'/, - }); - t.assert.strictEqual(stmt.setAllowBareNamedParameters(true), undefined); - t.assert.deepStrictEqual( - await stmt.run({ k: 3, v: 6 }), - { changes: 1, lastInsertRowid: 3 }, - ); - }); - - test('throws when input is not a boolean', async (t) => { - const db = new Database(nextDb()); - t.after(() => { db.close(); }); - const setup = await db.exec( - 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' - ); - t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); - t.assert.throws(() => { - stmt.setAllowBareNamedParameters(); - }, { - code: 'ERR_INVALID_ARG_TYPE', - message: /The "allowBareNamedParameters" argument must be a boolean/, - }); - }); -}); From 9a33662fc7248de427a759854becb0d140888278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Mon, 2 Mar 2026 22:17:57 +0000 Subject: [PATCH 13/15] sqlite: reject instead of throw on invalid state Async methods should always asynchronously reject with an error instead of throwing a synchronous error. Therefore convert the previous invalid state synchronous throws to asynchronous rejections for the Database and Statement classes. --- src/node_sqlite.cc | 42 ++++++++++++++------ test/parallel/test-sqlite-database-async.mjs | 20 +++++----- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 1e377d402390c4..240a543848c6a9 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -12,6 +12,7 @@ #include "threadpoolwork-inl.h" #include "util-inl.h" #include "uv.h" +#include "v8-isolate.h" #include #include @@ -84,6 +85,14 @@ using v8::Value; } \ } while (0) +#define REJECT_AND_RETURN_ON_INVALID_STATE(env, args, condition, msg) \ + do { \ + if ((condition)) { \ + RejectErrInvalidState((env), (args), (msg)); \ + return; \ + } \ + } while (0) + #define SQLITE_VALUE_TO_JS(from, isolate, use_big_int_args, result, ...) \ do { \ switch (sqlite3_##from##_type(__VA_ARGS__)) { \ @@ -296,6 +305,17 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, int errcode) { } } +inline void RejectErrInvalidState(Environment* env, + const FunctionCallbackInfo& args, + std::string_view message) { + Isolate* isolate = env->isolate(); + Local context = env->context(); + Local error = ERR_INVALID_STATE(isolate, message); + auto resolver = Promise::Resolver::New(context).ToLocalChecked(); + resolver->Reject(context, error).ToChecked(); + args.GetReturnValue().Set(resolver->GetPromise()); +} + inline void RejectWithSQLiteError(Environment* env, Local resolver, sqlite3* db) { @@ -4845,17 +4865,11 @@ void Database::Close(const v8::FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); - Local context = env->context(); if (!db->IsOpen()) { if (!db->IsDisposed()) { db->EnterDisposedStateSync(); } - auto resolver = Promise::Resolver::New(context).ToLocalChecked(); - resolver - ->Reject(context, - ERR_INVALID_STATE(env->isolate(), "database is not open")) - .Check(); - args.GetReturnValue().Set(resolver->GetPromise()); + RejectErrInvalidState(env, args, "database is not open"); return; } @@ -4897,7 +4911,8 @@ void Database::Prepare(const v8::FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); // TODO(BurningEnlightenment): these should be rejections - THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + REJECT_AND_RETURN_ON_INVALID_STATE( + env, args, !db->IsOpen(), "database is not open"); if (!args[0]->IsString()) { THROW_ERR_INVALID_ARG_TYPE(env->isolate(), @@ -4921,7 +4936,8 @@ void Database::Exec(const v8::FunctionCallbackInfo& args) { // TODO(BurningEnlightenment): these should be rejections ASSIGN_OR_RETURN_UNWRAP(&db, args.This()); Environment* env = Environment::GetCurrent(args); - THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + REJECT_AND_RETURN_ON_INVALID_STATE( + env, args, !db->IsOpen(), "database is not open"); if (!args[0]->IsString()) { THROW_ERR_INVALID_ARG_TYPE(env->isolate(), @@ -5028,8 +5044,8 @@ void Statement::Get(const v8::FunctionCallbackInfo& args) { Statement* stmt; ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); Environment* env = Environment::GetCurrent(args); - THROW_AND_RETURN_ON_BAD_STATE( - env, stmt->IsDisposed(), "statement is disposed"); + REJECT_AND_RETURN_ON_INVALID_STATE( + env, args, stmt->IsDisposed(), "statement is disposed"); transfer::value bind_arguments; if (args.Length() > 1) { @@ -5047,8 +5063,8 @@ void Statement::All(const v8::FunctionCallbackInfo& args) { Statement* stmt; ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); Environment* env = Environment::GetCurrent(args); - THROW_AND_RETURN_ON_BAD_STATE( - env, stmt->IsDisposed(), "statement is disposed"); + REJECT_AND_RETURN_ON_INVALID_STATE( + env, args, stmt->IsDisposed(), "statement is disposed"); transfer::value bind_arguments; if (args.Length() > 1) { diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index 772414415ef106..0b2c371ad08a98 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -441,11 +441,11 @@ suite('Database.prototype.prepare()', { timeout: 1000 }, () => { t.assert.ok(stmt instanceof Statement); }); - test('throws if database is not open', (t) => { + test('throws if database is not open', async (t) => { const db = new Database(nextDb(), { open: false }); - t.assert.throws(() => { - db.prepare(); + await t.assert.rejects(async () => { + await db.prepare(); }, { code: 'ERR_INVALID_STATE', message: /database is not open/, @@ -497,15 +497,15 @@ suite('Database.prototype.exec()', { timeout: 1000 }, () => { }); }); - test('throws if database is not open', (t) => { + test('throws if database is not open', async (t) => { const db = new Database(nextDb(), { open: false }); - t.assert.throws(() => { - db.exec(); - }, { - code: 'ERR_INVALID_STATE', - message: /database is not open/, - }); + await t.assert.rejects( + db.exec(), + { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); }); test('throws if sql is not a string', (t) => { From 808844382b3cb3dfcccd4b45f48600a51f3975de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Mon, 2 Mar 2026 22:50:34 +0000 Subject: [PATCH 14/15] sqlite: reject instead of throw on invalid args Async methods should always asynchronously reject with an error instead of throwing a synchronous error. Therefore convert the previous invalid args synchronous throws to asynchronous rejections for the Database and Statement classes. --- src/node_sqlite.cc | 66 ++++++++++++++++---- test/parallel/test-sqlite-database-async.mjs | 6 +- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 240a543848c6a9..12b90341db1b9e 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -93,6 +94,14 @@ using v8::Value; } \ } while (0) +#define REJECT_AND_RETURN_ON_INVALID_ARG_TYPE(env, args, condition, msg) \ + do { \ + if ((condition)) { \ + RejectErrInvalidArgType((env), (args), (msg)); \ + return; \ + } \ + } while (0) + #define SQLITE_VALUE_TO_JS(from, isolate, use_big_int_args, result, ...) \ do { \ switch (sqlite3_##from##_type(__VA_ARGS__)) { \ @@ -305,17 +314,42 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, int errcode) { } } -inline void RejectErrInvalidState(Environment* env, - const FunctionCallbackInfo& args, - std::string_view message) { - Isolate* isolate = env->isolate(); +inline void RejectErr(Environment* env, + const FunctionCallbackInfo& args, + Local error) { Local context = env->context(); - Local error = ERR_INVALID_STATE(isolate, message); auto resolver = Promise::Resolver::New(context).ToLocalChecked(); resolver->Reject(context, error).ToChecked(); args.GetReturnValue().Set(resolver->GetPromise()); } +template + requires requires(ErrFactory err_factory, Isolate* isolate) { + { err_factory(isolate) } -> std::convertible_to>; + } +inline void RejectErr(Environment* env, + const FunctionCallbackInfo& args, + ErrFactory&& err_factory) { + Local error = err_factory(env->isolate()); + RejectErr(env, args, error); +} + +inline void RejectErrInvalidState(Environment* env, + const FunctionCallbackInfo& args, + std::string_view message) { + RejectErr(env, args, [&](Isolate* isolate) -> Local { + return ERR_INVALID_STATE(isolate, message); + }); +} + +inline void RejectErrInvalidArgType(Environment* env, + const FunctionCallbackInfo& args, + std::string_view message) { + RejectErr(env, args, [&](Isolate* isolate) -> Local { + return ERR_INVALID_ARG_TYPE(isolate, message); + }); +} + inline void RejectWithSQLiteError(Environment* env, Local resolver, sqlite3* db) { @@ -4938,12 +4972,12 @@ void Database::Exec(const v8::FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); REJECT_AND_RETURN_ON_INVALID_STATE( env, args, !db->IsOpen(), "database is not open"); + REJECT_AND_RETURN_ON_INVALID_ARG_TYPE( + env, + args, + args.Length() < 1 || !args[0]->IsString(), + "The \"sql\" argument must be a string."); - if (!args[0]->IsString()) { - THROW_ERR_INVALID_ARG_TYPE(env->isolate(), - "The \"sql\" argument must be a string."); - return; - } Utf8Value sql(env->isolate(), args[0].As()); args.GetReturnValue().Set( db->Schedule(std::pmr::string(*sql, sql.length()))); @@ -5048,9 +5082,13 @@ void Statement::Get(const v8::FunctionCallbackInfo& args) { env, args, stmt->IsDisposed(), "statement is disposed"); transfer::value bind_arguments; - if (args.Length() > 1) { + if (args.Length() >= 1) { + TryCatch try_catch(env->isolate()); if (!transfer::ToValue(env->isolate(), args[0].As()) .MoveTo(&bind_arguments)) { + if (try_catch.HasCaught() && try_catch.CanContinue()) { + RejectErr(env, args, try_catch.Exception()); + } return; } } @@ -5067,9 +5105,13 @@ void Statement::All(const v8::FunctionCallbackInfo& args) { env, args, stmt->IsDisposed(), "statement is disposed"); transfer::value bind_arguments; - if (args.Length() > 1) { + if (args.Length() >= 1) { + TryCatch try_catch(env->isolate()); if (!transfer::ToValue(env->isolate(), args[0].As()) .MoveTo(&bind_arguments)) { + if (try_catch.HasCaught() && try_catch.CanContinue()) { + RejectErr(env, args, try_catch.Exception()); + } return; } } diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index 0b2c371ad08a98..0b96c53b4233cf 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -508,12 +508,12 @@ suite('Database.prototype.exec()', { timeout: 1000 }, () => { }); }); - test('throws if sql is not a string', (t) => { + test('rejects if sql is not a string', async (t) => { const db = new Database(nextDb()); t.after(async () => { await db.close(); }); - t.assert.throws(() => { - db.exec(); + await t.assert.rejects(async () => { + await db.exec(); }, { code: 'ERR_INVALID_ARG_TYPE', message: /The "sql" argument must be a string/, From d50bd9b4d9b11e045463f4112f40fd19a6168a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Steffen=20Ga=C3=9Fmann?= Date: Mon, 2 Mar 2026 23:35:02 +0000 Subject: [PATCH 15/15] sqlite: implement `Statement.prototype.run()` --- src/node_sqlite.cc | 184 +++++++++++++++---- src/node_sqlite.h | 1 + test/parallel/test-sqlite-database-async.mjs | 4 +- test/parallel/test-sqlite-statement-async.js | 38 ++-- 4 files changed, 167 insertions(+), 60 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 12b90341db1b9e..d29ee79f17f292 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -13,6 +13,9 @@ #include "util-inl.h" #include "uv.h" #include "v8-isolate.h" +#include "v8-local-handle.h" +#include "v8-maybe.h" +#include "v8-template.h" #include #include @@ -2853,6 +2856,49 @@ MaybeLocal StatementExecutionHelper::All(Environment* env, return scope.Escape(Array::New(isolate, rows.data(), rows.size())); } +namespace { +inline Local GetRunResultTemplate(Environment* env, + Isolate* isolate) { + auto run_result_template = env->sqlite_run_result_template(); + if (run_result_template.IsEmpty()) { + static constexpr std::string_view run_result_keys[] = {"changes", + "lastInsertRowid"}; + run_result_template = DictionaryTemplate::New(isolate, run_result_keys); + env->set_sqlite_run_result_template(run_result_template); + } + return run_result_template; +} + +inline MaybeLocal CreateRunResultObject(Environment* env, + sqlite3_int64 changes, + sqlite3_int64 last_insert_rowid, + bool use_big_ints) { + Isolate* isolate = env->isolate(); + Local context = env->context(); + Local changes_val; + Local last_insert_rowid_val; + + if (use_big_ints) { + changes_val = BigInt::New(isolate, changes); + last_insert_rowid_val = BigInt::New(isolate, last_insert_rowid); + } else { + changes_val = Number::New(isolate, changes); + last_insert_rowid_val = Number::New(isolate, last_insert_rowid); + } + + auto run_result_template = GetRunResultTemplate(env, isolate); + + MaybeLocal values[] = {changes_val, last_insert_rowid_val}; + Local result; + if (!NewDictionaryInstance(context, run_result_template, values) + .ToLocal(&result)) { + return {}; + } + + return result; +} +} // namespace + MaybeLocal StatementExecutionHelper::Run(Environment* env, DatabaseSync* db, sqlite3_stmt* stmt, @@ -2865,28 +2911,9 @@ MaybeLocal StatementExecutionHelper::Run(Environment* env, sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(db->Connection()); sqlite3_int64 changes = sqlite3_changes64(db->Connection()); - Local last_insert_rowid_val; - Local changes_val; - - if (use_big_ints) { - last_insert_rowid_val = BigInt::New(isolate, last_insert_rowid); - changes_val = BigInt::New(isolate, changes); - } else { - last_insert_rowid_val = Number::New(isolate, last_insert_rowid); - changes_val = Number::New(isolate, changes); - } - - auto run_result_template = env->sqlite_run_result_template(); - if (run_result_template.IsEmpty()) { - static constexpr std::string_view run_result_keys[] = {"changes", - "lastInsertRowid"}; - run_result_template = DictionaryTemplate::New(isolate, run_result_keys); - env->set_sqlite_run_result_template(run_result_template); - } - MaybeLocal values[] = {changes_val, last_insert_rowid_val}; Local result; - if (!NewDictionaryInstance(env->context(), run_result_template, values) + if (!CreateRunResultObject(env, changes, last_insert_rowid, use_big_ints) .ToLocal(&result)) { return MaybeLocal(); } @@ -4147,9 +4174,11 @@ class alignas(64) OperationResult { return Rejected::ErrorCode(error_code, error_message); } - void Connect(Isolate* isolate, + void Connect(Environment* env, Local context, Promise::Resolver* resolver) const { + Isolate* isolate = env->isolate(); + // Use ToLocalChecked here because we are trying to report a failure and // failing to report a failure would be worse than crashing as it would be // an API contract violation. @@ -4201,10 +4230,10 @@ class alignas(64) OperationResult { }; class Void { public: - void Connect(Isolate* isolate, + void Connect(Environment* env, Local context, Promise::Resolver* resolver) const { - resolver->Resolve(context, Undefined(isolate)).Check(); + resolver->Resolve(context, Undefined(env->isolate())).Check(); } }; class PreparedStatement { @@ -4212,9 +4241,10 @@ class alignas(64) OperationResult { explicit PreparedStatement(BaseObjectPtr db, sqlite3_stmt* stmt) : db_(std::move(db)), stmt_(stmt) {} - void Connect(Isolate* isolate, + void Connect(Environment* env, Local context, Promise::Resolver* resolver) const { + Isolate* isolate = env->isolate(); auto* stmt = stmt_; if (!db_->IsOpen()) { // Database is closing, therefore directly create a disposed Statement. @@ -4245,10 +4275,10 @@ class alignas(64) OperationResult { public: explicit Value(transfer::value value) : value_(std::move(value)) {} - void Connect(Isolate* isolate, + void Connect(Environment* env, Local context, Promise::Resolver* resolver) const { - resolver->Resolve(context, transfer::to_v8_value(isolate)(value_)) + resolver->Resolve(context, transfer::to_v8_value(env->isolate())(value_)) .Check(); } @@ -4260,9 +4290,10 @@ class alignas(64) OperationResult { explicit Values(std::pmr::vector values) : values_(std::move(values)) {} - void Connect(Isolate* isolate, + void Connect(Environment* env, Local context, Promise::Resolver* resolver) const { + Isolate* isolate = env->isolate(); CHECK_LE(values_.size(), static_cast(std::numeric_limits::max())); int size = static_cast(values_.size()); @@ -4277,9 +4308,27 @@ class alignas(64) OperationResult { private: std::pmr::vector values_; }; + class RunResult { + public: + RunResult(sqlite3_int64 changes, sqlite3_int64 last_insert_rowid) + : changes_(changes), last_insert_rowid_(last_insert_rowid) {} + + void Connect(Environment* env, + Local context, + Promise::Resolver* resolver) const { + auto result = + CreateRunResultObject(env, changes_, last_insert_rowid_, false) + .ToLocalChecked(); + resolver->Resolve(context, result).Check(); + } + + private: + sqlite3_int64 changes_; + sqlite3_int64 last_insert_rowid_; + }; using variant_type = - std::variant; + std::variant; public: static OperationResult RejectErrorCode(OperationBase* origin, @@ -4301,6 +4350,11 @@ class alignas(64) OperationResult { OperationBase* origin, std::pmr::vector&& values) { return OperationResult{origin, Values{std::move(values)}}; } + static OperationResult ResolveRunResult(OperationBase* origin, + sqlite3_int64 changes, + sqlite3_int64 last_insert_rowid) { + return OperationResult{origin, RunResult{changes, last_insert_rowid}}; + } static OperationResult ResolvePreparedStatement(OperationBase* origin, BaseObjectPtr db, sqlite3_stmt* stmt) { @@ -4312,14 +4366,12 @@ class alignas(64) OperationResult { OperationResult(OperationBase* origin, T&& value) : origin_(origin), result_(std::forward(value)) {} - void Connect(Isolate* isolate) const { - Local context = isolate->GetCurrentContext(); - Local resolver = origin_->GetResolver(isolate); - std::visit( - [isolate, &context, &resolver](auto&& value) { - value.Connect(isolate, context, *resolver); - }, - result_); + void Connect(Environment* env) const { + Local context = env->context(); + Local resolver = origin_->GetResolver(env->isolate()); + std::visit([env, &context, &resolver]( + auto&& value) { value.Connect(env, context, *resolver); }, + result_); } private: @@ -4450,6 +4502,35 @@ class StatementAllOperation : private OperationBase { transfer::value bind_arguments_; }; +class StatementRunOperation : private OperationBase { + public: + StatementRunOperation(Global&& resolver, + sqlite3_stmt* stmt, + transfer::value&& bind_arguments) + : OperationBase(std::move(resolver)), + stmt_(stmt), + bind_arguments_(std::move(bind_arguments)) {} + + OperationResult operator()(sqlite3* connection) { + auto clear_bindings = OnScopeLeave([&] { sqlite3_clear_bindings(stmt_); }); + if (int r = transfer::bind_value(stmt_)(bind_arguments_); r != SQLITE_OK) { + return OperationResult::RejectErrorCode(this, r); + } + (void)sqlite3_step(stmt_); + if (sqlite3_reset(stmt_) != SQLITE_OK) { + return OperationResult::RejectLastError(this, connection); + } + + sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(connection); + sqlite3_int64 changes = sqlite3_changes(connection); + return OperationResult::ResolveRunResult(this, changes, last_insert_rowid); + } + + private: + sqlite3_stmt* stmt_; + transfer::value bind_arguments_; +}; + class ExecOperation : private OperationBase { public: ExecOperation(Global&& resolver, std::pmr::string&& sql) @@ -4488,6 +4569,7 @@ class CloseOperation : private OperationBase { using Operation = std::variant; @@ -4662,11 +4744,11 @@ class DatabaseOperationExecutor final : private ThreadPoolWork { if (results.empty()) { return; } - Isolate* isolate = env()->isolate(); - HandleScope handle_scope(isolate); + Environment* env = this->env(); + HandleScope handle_scope(env->isolate()); for (OperationResult& result : results) { - result.Connect(isolate); + result.Connect(env); } } void ScheduleWork(DatabaseOperationQueue* batch) { @@ -5026,6 +5108,7 @@ Local Statement::GetConstructorTemplate(Environment* env) { Statement::IsDisposedGetter); SetProtoMethod(isolate, tmpl, "get", Statement::Get); SetProtoMethod(isolate, tmpl, "all", Statement::All); + SetProtoMethod(isolate, tmpl, "run", Statement::Run); env->set_sqlite_statement_constructor_template(tmpl); } @@ -5120,6 +5203,29 @@ void Statement::All(const v8::FunctionCallbackInfo& args) { stmt->statement_, std::move(bind_arguments))); } +void Statement::Run(const v8::FunctionCallbackInfo& args) { + Statement* stmt; + ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); + Environment* env = Environment::GetCurrent(args); + REJECT_AND_RETURN_ON_INVALID_STATE( + env, args, stmt->IsDisposed(), "statement is disposed"); + + transfer::value bind_arguments; + if (args.Length() >= 1) { + TryCatch try_catch(env->isolate()); + if (!transfer::ToValue(env->isolate(), args[0].As()) + .MoveTo(&bind_arguments)) { + if (try_catch.HasCaught() && try_catch.CanContinue()) { + RejectErr(env, args, try_catch.Exception()); + } + return; + } + } + + args.GetReturnValue().Set(stmt->db_->Schedule( + stmt->statement_, std::move(bind_arguments))); +} + void DefineConstants(Local target) { NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_OMIT); NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_REPLACE); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 73abf9a0b7b76f..a71d4652de5672 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -531,6 +531,7 @@ class Statement final : public BaseObject { static void Get(const v8::FunctionCallbackInfo& args); static void All(const v8::FunctionCallbackInfo& args); + static void Run(const v8::FunctionCallbackInfo& args); SET_MEMORY_INFO_NAME(Statement) SET_SELF_SIZE(Statement) diff --git a/test/parallel/test-sqlite-database-async.mjs b/test/parallel/test-sqlite-database-async.mjs index 0b96c53b4233cf..f25b7a0739d53a 100644 --- a/test/parallel/test-sqlite-database-async.mjs +++ b/test/parallel/test-sqlite-database-async.mjs @@ -275,7 +275,7 @@ suite('Database() constructor', { timeout: 1000 }, () => { }); }); - test.skip('allows unknown named parameters', async (t) => { + test('allows unknown named parameters', async (t) => { const dbPath = nextDb(); const db = new Database(dbPath, { allowUnknownNamedParameters: true }); t.after(async () => { await db.close(); }); @@ -284,7 +284,7 @@ suite('Database() constructor', { timeout: 1000 }, () => { ); t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); + using stmt = await db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)'); const params = { $a: 1, $b: 2, $k: 42, $y: 25, $v: 84, $z: 99 }; t.assert.deepStrictEqual( await stmt.run(params), diff --git a/test/parallel/test-sqlite-statement-async.js b/test/parallel/test-sqlite-statement-async.js index 9b8e9aad39cad4..550363fb252547 100644 --- a/test/parallel/test-sqlite-statement-async.js +++ b/test/parallel/test-sqlite-statement-async.js @@ -189,29 +189,29 @@ suite.skip('Statement.prototype.iterate()', () => { }); }); -suite.skip('Statement.prototype.run()', () => { +suite('Statement.prototype.run()', () => { test('executes a query and returns change metadata', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec(` CREATE TABLE storage(key TEXT, val TEXT); INSERT INTO storage (key, val) VALUES ('foo', 'bar'); `); t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('SELECT * FROM storage'); + using stmt = await db.prepare('SELECT * FROM storage'); t.assert.deepStrictEqual(await stmt.run(), { changes: 1, lastInsertRowid: 1 }); }); - test('SQLite throws when trying to bind too many parameters', async (t) => { + test.skip('SQLite throws when trying to bind too many parameters', async (t) => { const db = new Database(nextDb()); t.after(() => { db.close(); }); const setup = await db.exec( 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;' ); t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?, ?)'); + using stmt = await db.prepare('INSERT INTO data (key, val) VALUES (?, ?)'); t.assert.throws(() => { - stmt.run(1, 2, 3); + stmt.run([1, 2, 3]); }, { code: 'ERR_SQLITE_ERROR', message: 'column index out of range', @@ -222,14 +222,14 @@ suite.skip('Statement.prototype.run()', () => { test('SQLite defaults to NULL for unbound parameters', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec( 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;' ); t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?, ?)'); + using stmt = await db.prepare('INSERT INTO data (key, val) VALUES (?, ?)'); await t.assert.rejects( - stmt.run(1), + stmt.run([1]), { code: 'ERR_SQLITE_ERROR', message: 'NOT NULL constraint failed: data.val', @@ -245,29 +245,29 @@ suite.skip('Statement.prototype.run()', () => { ); t.assert.strictEqual(setup, undefined); const sql = 'INSERT INTO data (key, val) VALUES ($k, $v) RETURNING key'; - const stmt = db.prepare(sql); + using stmt = await db.prepare(sql); t.assert.deepStrictEqual( - await stmt.run({ k: 1, v: 10 }), { changes: 1, lastInsertRowid: 1 } + await stmt.run({ $k: 1, $v: 10 }), { changes: 1, lastInsertRowid: 1 } ); t.assert.deepStrictEqual( - await stmt.run({ k: 2, v: 20 }), { changes: 1, lastInsertRowid: 2 } + await stmt.run({ $k: 2, $v: 20 }), { changes: 1, lastInsertRowid: 2 } ); t.assert.deepStrictEqual( - await stmt.run({ k: 3, v: 30 }), { changes: 1, lastInsertRowid: 3 } + await stmt.run({ $k: 3, $v: 30 }), { changes: 1, lastInsertRowid: 3 } ); }); test('SQLite defaults unbound ?NNN parameters', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec( 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;' ); t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?1, ?3)'); + using stmt = await db.prepare('INSERT INTO data (key, val) VALUES (?1, ?3)'); await t.assert.rejects( - stmt.run(1), + stmt.run([1]), { code: 'ERR_SQLITE_ERROR', message: 'NOT NULL constraint failed: data.val', @@ -278,13 +278,13 @@ suite.skip('Statement.prototype.run()', () => { test('binds ?NNN params by position', async (t) => { const db = new Database(nextDb()); - t.after(() => { db.close(); }); + t.after(async () => { await db.close(); }); const setup = await db.exec( 'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;' ); t.assert.strictEqual(setup, undefined); - const stmt = db.prepare('INSERT INTO data (key, val) VALUES (?1, ?2)'); - t.assert.deepStrictEqual(await stmt.run(1, 2), { changes: 1, lastInsertRowid: 1 }); + using stmt = await db.prepare('INSERT INTO data (key, val) VALUES (?1, ?2)'); + t.assert.deepStrictEqual(await stmt.run([1, 2]), { changes: 1, lastInsertRowid: 1 }); }); });