diff --git a/src/env_properties.h b/src/env_properties.h index 8db1ba8a2f7..9e950c50791 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -448,6 +448,7 @@ V(shutdown_wrap_template, v8::ObjectTemplate) \ V(socketaddress_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) \ V(streambaseentry_ctor_template, v8::FunctionTemplate) \ V(streambaseoutputstream_constructor_template, v8::ObjectTemplate) \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 520d54818d5..1f4395d9a58 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -25,7 +25,6 @@ using v8::ConstructorBehavior; using v8::Context; using v8::DontDelete; using v8::Exception; -using v8::External; using v8::Function; using v8::FunctionCallback; using v8::FunctionCallbackInfo; @@ -1620,142 +1619,12 @@ void StatementSync::All(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size())); } -void StatementSync::IterateReturnCallback( - const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - auto isolate = env->isolate(); - auto context = isolate->GetCurrentContext(); - - auto self = args.This(); - // iterator has fetch all result or break, prevent next func to return result - if (self->Set(context, env->isfinished_string(), Boolean::New(isolate, true)) - .IsNothing()) { - // An error will have been scheduled. - return; - } - - Local val; - if (!self->Get(context, env->statement_string()).ToLocal(&val)) { - // An error will have been scheduled. - return; - } - auto external_stmt = Local::Cast(val); - auto stmt = static_cast(external_stmt->Value()); - if (!stmt->IsFinalized()) { - sqlite3_reset(stmt->statement_); - } - - LocalVector keys(isolate, {env->done_string(), env->value_string()}); - LocalVector values(isolate, - {Boolean::New(isolate, true), Null(isolate)}); - - DCHECK_EQ(keys.size(), values.size()); - Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), keys.size()); - args.GetReturnValue().Set(result); -} - -void StatementSync::IterateNextCallback( - const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - auto isolate = env->isolate(); - auto context = isolate->GetCurrentContext(); - - auto self = args.This(); - - Local val; - if (!self->Get(context, env->isfinished_string()).ToLocal(&val)) { - // An error will have been scheduled. - return; - } - - // skip iteration if is_finished - auto is_finished = Local::Cast(val); - if (is_finished->Value()) { - Local keys[] = {env->done_string(), env->value_string()}; - Local values[] = {Boolean::New(isolate, true), Null(isolate)}; - static_assert(arraysize(keys) == arraysize(values)); - Local result = Object::New( - isolate, Null(isolate), &keys[0], &values[0], arraysize(keys)); - args.GetReturnValue().Set(result); - return; - } - - if (!self->Get(context, env->statement_string()).ToLocal(&val)) { - // An error will have been scheduled. - return; - } - - auto external_stmt = Local::Cast(val); - auto stmt = static_cast(external_stmt->Value()); - - if (!self->Get(context, env->num_cols_string()).ToLocal(&val)) { - // An error will have been scheduled. - return; - } - - auto num_cols = Local::Cast(val)->Value(); - - THROW_AND_RETURN_ON_BAD_STATE( - env, stmt->IsFinalized(), "statement has been finalized"); - - int r = sqlite3_step(stmt->statement_); - if (r != SQLITE_ROW) { - CHECK_ERROR_OR_THROW( - env->isolate(), stmt->db_.get(), r, SQLITE_DONE, void()); - - // cleanup when no more rows to fetch - sqlite3_reset(stmt->statement_); - if (self->Set( - context, env->isfinished_string(), Boolean::New(isolate, true)) - .IsNothing()) { - // An error would have been scheduled - return; - } - - LocalVector keys(isolate, {env->done_string(), env->value_string()}); - LocalVector values(isolate, - {Boolean::New(isolate, true), Null(isolate)}); - - DCHECK_EQ(keys.size(), values.size()); - Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), keys.size()); - args.GetReturnValue().Set(result); - return; - } - - LocalVector row_keys(isolate); - row_keys.reserve(num_cols); - LocalVector row_values(isolate); - row_values.reserve(num_cols); - for (int i = 0; i < num_cols; ++i) { - Local key; - if (!stmt->ColumnNameToName(i).ToLocal(&key)) return; - Local val; - if (!stmt->ColumnToValue(i).ToLocal(&val)) return; - row_keys.emplace_back(key); - row_values.emplace_back(val); - } - - Local row = Object::New( - isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); - - LocalVector keys(isolate, {env->done_string(), env->value_string()}); - LocalVector values(isolate, {Boolean::New(isolate, false), row}); - - DCHECK_EQ(keys.size(), values.size()); - Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), keys.size()); - args.GetReturnValue().Set(result); -} - void StatementSync::Iterate(const FunctionCallbackInfo& args) { StatementSync* stmt; ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); - auto isolate = env->isolate(); auto context = env->context(); int r = sqlite3_reset(stmt->statement_); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); @@ -1764,67 +1633,28 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { return; } - Local next_func; - Local return_func; - if (!Function::New(context, StatementSync::IterateNextCallback) - .ToLocal(&next_func) || - !Function::New(context, StatementSync::IterateReturnCallback) - .ToLocal(&return_func)) { - // An error will have been scheduled. - return; - } - - Local keys[] = {env->next_string(), env->return_string()}; - Local values[] = {next_func, return_func}; - static_assert(arraysize(keys) == arraysize(values)); - Local global = context->Global(); Local js_iterator; Local js_iterator_prototype; - if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator)) + if (!global->Get(context, env->iterator_string()).ToLocal(&js_iterator)) { return; + } if (!js_iterator.As() ->Get(context, env->prototype_string()) - .ToLocal(&js_iterator_prototype)) - return; - - Local iterable_iterator = Object::New( - isolate, js_iterator_prototype, &keys[0], &values[0], arraysize(keys)); - - auto num_cols_pd = v8::PropertyDescriptor( - v8::Integer::New(isolate, sqlite3_column_count(stmt->statement_)), false); - num_cols_pd.set_enumerable(false); - num_cols_pd.set_configurable(false); - if (iterable_iterator - ->DefineProperty(context, env->num_cols_string(), num_cols_pd) - .IsNothing()) { - // An error will have been scheduled. + .ToLocal(&js_iterator_prototype)) { return; } - auto stmt_pd = - v8::PropertyDescriptor(v8::External::New(isolate, stmt), false); - stmt_pd.set_enumerable(false); - stmt_pd.set_configurable(false); - if (iterable_iterator - ->DefineProperty(context, env->statement_string(), stmt_pd) + BaseObjectPtr iter = + StatementSyncIterator::Create(env, BaseObjectPtr(stmt)); + if (iter->object() + ->GetPrototype() + .As() + ->SetPrototype(context, js_iterator_prototype) .IsNothing()) { - // An error will have been scheduled. return; } - - auto is_finished_pd = - v8::PropertyDescriptor(v8::Boolean::New(isolate, false), true); - stmt_pd.set_enumerable(false); - stmt_pd.set_configurable(false); - if (iterable_iterator - ->DefineProperty(context, env->isfinished_string(), is_finished_pd) - .IsNothing()) { - // An error will have been scheduled. - return; - } - - args.GetReturnValue().Set(iterable_iterator); + args.GetReturnValue().Set(iter->object()); } void StatementSync::Get(const FunctionCallbackInfo& args) { @@ -2144,6 +1974,118 @@ BaseObjectPtr StatementSync::Create( return MakeBaseObject(env, obj, std::move(db), stmt); } +StatementSyncIterator::StatementSyncIterator(Environment* env, + Local object, + BaseObjectPtr stmt) + : BaseObject(env, object), stmt_(std::move(stmt)) { + MakeWeak(); + done_ = false; +} + +StatementSyncIterator::~StatementSyncIterator() {} +void StatementSyncIterator::MemoryInfo(MemoryTracker* tracker) const {} + +Local StatementSyncIterator::GetConstructorTemplate( + Environment* env) { + Local tmpl = + env->sqlite_statement_sync_iterator_constructor_template(); + if (tmpl.IsEmpty()) { + Isolate* isolate = env->isolate(); + tmpl = NewFunctionTemplate(isolate, IllegalConstructor); + tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "StatementSyncIterator")); + tmpl->InstanceTemplate()->SetInternalFieldCount( + StatementSync::kInternalFieldCount); + SetProtoMethod(isolate, tmpl, "next", StatementSyncIterator::Next); + SetProtoMethod(isolate, tmpl, "return", StatementSyncIterator::Return); + env->set_sqlite_statement_sync_iterator_constructor_template(tmpl); + } + return tmpl; +} + +BaseObjectPtr StatementSyncIterator::Create( + Environment* env, BaseObjectPtr stmt) { + Local obj; + if (!GetConstructorTemplate(env) + ->InstanceTemplate() + ->NewInstance(env->context()) + .ToLocal(&obj)) { + return BaseObjectPtr(); + } + + return MakeBaseObject(env, obj, std::move(stmt)); +} + +void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { + StatementSyncIterator* iter; + ASSIGN_OR_RETURN_UNWRAP(&iter, args.This()); + Environment* env = Environment::GetCurrent(args); + THROW_AND_RETURN_ON_BAD_STATE( + env, iter->stmt_->IsFinalized(), "statement has been finalized"); + Isolate* isolate = env->isolate(); + LocalVector keys(isolate, {env->done_string(), env->value_string()}); + + if (iter->done_) { + LocalVector values(isolate, + {Boolean::New(isolate, true), Null(isolate)}); + Local result = Object::New( + isolate, Null(isolate), keys.data(), values.data(), keys.size()); + args.GetReturnValue().Set(result); + return; + } + + int r = sqlite3_step(iter->stmt_->statement_); + if (r != SQLITE_ROW) { + CHECK_ERROR_OR_THROW( + env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void()); + sqlite3_reset(iter->stmt_->statement_); + LocalVector values(isolate, + {Boolean::New(isolate, true), Null(isolate)}); + Local result = Object::New( + isolate, Null(isolate), keys.data(), values.data(), keys.size()); + args.GetReturnValue().Set(result); + return; + } + + int num_cols = sqlite3_column_count(iter->stmt_->statement_); + LocalVector row_keys(isolate); + LocalVector row_values(isolate); + row_keys.reserve(num_cols); + row_values.reserve(num_cols); + for (int i = 0; i < num_cols; ++i) { + Local key; + if (!iter->stmt_->ColumnNameToName(i).ToLocal(&key)) return; + Local val; + if (!iter->stmt_->ColumnToValue(i).ToLocal(&val)) return; + row_keys.emplace_back(key); + row_values.emplace_back(val); + } + + Local row = Object::New( + isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); + LocalVector values(isolate, {Boolean::New(isolate, false), row}); + Local result = Object::New( + isolate, Null(isolate), keys.data(), values.data(), keys.size()); + args.GetReturnValue().Set(result); +} + +void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { + StatementSyncIterator* iter; + ASSIGN_OR_RETURN_UNWRAP(&iter, args.This()); + Environment* env = Environment::GetCurrent(args); + THROW_AND_RETURN_ON_BAD_STATE( + env, iter->stmt_->IsFinalized(), "statement has been finalized"); + Isolate* isolate = env->isolate(); + + sqlite3_reset(iter->stmt_->statement_); + iter->done_ = true; + LocalVector keys(isolate, {env->done_string(), env->value_string()}); + LocalVector values(isolate, + {Boolean::New(isolate, true), Null(isolate)}); + Local result = Object::New( + isolate, Null(isolate), keys.data(), values.data(), keys.size()); + args.GetReturnValue().Set(result); +} + Session::Session(Environment* env, Local object, BaseObjectWeakPtr database, diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 94abd22b857..9e8b4aa33da 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -145,10 +145,29 @@ class StatementSync : public BaseObject { v8::MaybeLocal ColumnToValue(const int column); v8::MaybeLocal ColumnNameToName(const int column); - static void IterateNextCallback( - const v8::FunctionCallbackInfo& args); - static void IterateReturnCallback( - const v8::FunctionCallbackInfo& args); + friend class StatementSyncIterator; +}; + +class StatementSyncIterator : public BaseObject { + public: + StatementSyncIterator(Environment* env, + v8::Local object, + BaseObjectPtr stmt); + void MemoryInfo(MemoryTracker* tracker) const override; + static v8::Local GetConstructorTemplate( + Environment* env); + static BaseObjectPtr Create( + Environment* env, BaseObjectPtr stmt); + static void Next(const v8::FunctionCallbackInfo& args); + static void Return(const v8::FunctionCallbackInfo& args); + + SET_MEMORY_INFO_NAME(StatementSyncIterator) + SET_SELF_SIZE(StatementSyncIterator) + + private: + ~StatementSyncIterator() override; + BaseObjectPtr stmt_; + bool done_; }; using Sqlite3ChangesetGenFunc = int (*)(sqlite3_session*, int*, void**); diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index 62641823f27..33a50188b5f 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -1,3 +1,4 @@ +// Flags: --expose-gc 'use strict'; require('../common'); const tmpdir = require('../common/tmpdir'); @@ -88,7 +89,10 @@ suite('StatementSync.prototype.iterate()', () => { const db = new DatabaseSync(nextDb()); t.after(() => { db.close(); }); const stmt = db.prepare('CREATE TABLE storage(key TEXT, val TEXT)'); - t.assert.deepStrictEqual(stmt.iterate().toArray(), []); + 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', (t) => { @@ -119,6 +123,53 @@ suite('StatementSync.prototype.iterate()', () => { t.assert.deepStrictEqual(item, itemsLoop.shift()); } }); + + test('iterator keeps the prepared statement from being collected', (t) => { + const db = new DatabaseSync(':memory:'); + 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', (t) => { + const db = new DatabaseSync(':memory:'); + 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('StatementSync.prototype.run()', () => {