From 0111ab75a8a5b61bc3cfc676f611e0030cf72437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 01:49:15 +0100 Subject: [PATCH 01/15] sqlite: add setReturnArrays method to StatementSync --- src/node_sqlite.cc | 263 ++++++++++++++++++-- src/node_sqlite.h | 2 + test/parallel/test-sqlite-statement-sync.js | 124 +++++++++ 3 files changed, 364 insertions(+), 25 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 36b8877a27f733..c12c65a1462e17 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1568,31 +1568,188 @@ void StatementSync::All(const FunctionCallbackInfo& args) { int num_cols = sqlite3_column_count(stmt->statement_); LocalVector rows(isolate); LocalVector row_keys(isolate); + while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) { - if (row_keys.size() == 0) { - row_keys.reserve(num_cols); + if (stmt->return_arrays_) { + Local row = Array::New(isolate, num_cols); + for (int i = 0; i < num_cols; ++i) { + Local val; + if (!stmt->ColumnToValue(i).ToLocal(&val)) return; + if (row->Set(env->context(), i, val).IsNothing()) return; + } + rows.emplace_back(row); + } else { + if (row_keys.size() == 0) { + row_keys.reserve(num_cols); + for (int i = 0; i < num_cols; ++i) { + Local key; + if (!stmt->ColumnNameToName(i).ToLocal(&key)) return; + row_keys.emplace_back(key); + } + } + + 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; - row_keys.emplace_back(key); + Local val; + if (!stmt->ColumnToValue(i).ToLocal(&val)) return; + row_values.emplace_back(val); } + + Local row = Object::New( + isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); + rows.emplace_back(row); } + } + + CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_DONE, void()); + 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 v8::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"); + + // Get the return_arrays flag from the iterator object + Local return_arrays_val; + bool return_arrays = false; + if (self->Get(context, FIXED_ONE_BYTE_STRING(isolate, "return_arrays")) + .ToLocal(&return_arrays_val) && + return_arrays_val->IsBoolean()) { + return_arrays = return_arrays_val.As()->Value(); + } + + 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; + } + + 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; + } + + Local row; + if (return_arrays) { + Local array_row = Array::New(isolate, num_cols); + for (int i = 0; i < num_cols; ++i) { + Local val; + if (!stmt->ColumnToValue(i).ToLocal(&val)) return; + if (array_row->Set(env->context(), i, val).IsNothing()) return; + } + row = array_row; + } else { + 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( + row = Object::New( isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); - rows.emplace_back(row); } - CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_DONE, void()); - args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size())); + Local keys[] = {env->done_string(), env->value_string()}; + Local values[] = {Boolean::New(isolate, false), row}; + static_assert(arraysize(keys) == arraysize(values)); + Local result = Object::New( + isolate, Null(isolate), &keys[0], &values[0], arraysize(keys)); + args.GetReturnValue().Set(result); } void StatementSync::Iterate(const FunctionCallbackInfo& args) { @@ -1630,7 +1787,34 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { .IsNothing()) { return; } - args.GetReturnValue().Set(iter->object()); + + auto is_finished_pd = + v8::PropertyDescriptor(v8::Boolean::New(isolate, false), true); + is_finished_pd.set_enumerable(false); + is_finished_pd.set_configurable(false); + if (iterable_iterator + ->DefineProperty(context, env->isfinished_string(), is_finished_pd) + .IsNothing()) { + // An error will have been scheduled. + return; + } + + // Add the return_arrays flag to the iterator + auto return_arrays_pd = + v8::PropertyDescriptor(v8::Boolean::New( + isolate, stmt->return_arrays_), false); + return_arrays_pd.set_enumerable(false); + return_arrays_pd.set_configurable(false); + if (iterable_iterator + ->DefineProperty( + context, FIXED_ONE_BYTE_STRING( + isolate, "return_arrays"), return_arrays_pd) + .IsNothing()) { + // An error will have been scheduled. + return; + } + + args.GetReturnValue().Set(iterable_iterator); } void StatementSync::Get(const FunctionCallbackInfo& args) { @@ -1660,24 +1844,35 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { return; } - LocalVector keys(isolate); - keys.reserve(num_cols); - LocalVector values(isolate); - values.reserve(num_cols); + if (stmt->return_arrays_) { + Local result = Array::New(isolate, num_cols); + for (int i = 0; i < num_cols; ++i) { + Local val; + if (!stmt->ColumnToValue(i).ToLocal(&val)) return; + if (result->Set(env->context(), i, val).IsNothing()) return; + } + args.GetReturnValue().Set(result); + } else { + LocalVector keys(isolate); + keys.reserve(num_cols); + LocalVector values(isolate); + 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; - keys.emplace_back(key); - values.emplace_back(val); - } + 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; + keys.emplace_back(key); + values.emplace_back(val); + } - Local result = - Object::New(isolate, Null(isolate), keys.data(), values.data(), num_cols); + Local result = + Object::New( + isolate, Null(isolate), keys.data(), values.data(), num_cols); - args.GetReturnValue().Set(result); + args.GetReturnValue().Set(result); + } } void StatementSync::Run(const FunctionCallbackInfo& args) { @@ -1877,6 +2072,22 @@ void StatementSync::SetReadBigInts(const FunctionCallbackInfo& args) { stmt->use_big_ints_ = args[0]->IsTrue(); } +void StatementSync::SetReturnArrays(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"); + + if (!args[0]->IsBoolean()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), "The \"returnArrays\" argument must be a boolean."); + return; + } + + stmt->return_arrays_ = args[0]->IsTrue(); +} + void IllegalConstructor(const FunctionCallbackInfo& args) { THROW_ERR_ILLEGAL_CONSTRUCTOR(Environment::GetCurrent(args)); } @@ -1932,6 +2143,8 @@ Local StatementSync::GetConstructorTemplate( StatementSync::SetAllowUnknownNamedParameters); SetProtoMethod( isolate, tmpl, "setReadBigInts", StatementSync::SetReadBigInts); + SetProtoMethod( + isolate, tmpl, "setReturnArrays", StatementSync::SetReturnArrays); env->set_sqlite_statement_sync_constructor_template(tmpl); } return tmpl; diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 9e8b4aa33da1f8..98d39b6d371943 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -126,6 +126,7 @@ class StatementSync : public BaseObject { static void SetAllowUnknownNamedParameters( const v8::FunctionCallbackInfo& args); static void SetReadBigInts(const v8::FunctionCallbackInfo& args); + static void SetReturnArrays(const v8::FunctionCallbackInfo& args); void Finalize(); bool IsFinalized(); @@ -136,6 +137,7 @@ class StatementSync : public BaseObject { ~StatementSync() override; BaseObjectPtr db_; sqlite3_stmt* statement_; + bool return_arrays_ = false; bool use_big_ints_; bool allow_bare_named_params_; bool allow_unknown_named_params_; diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index 49a481c3922c1e..f13c51a81d22e5 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -343,6 +343,130 @@ suite('StatementSync.prototype.setReadBigInts()', () => { }); }); +suite('StatementSync.prototype.setReturnArrays()', () => { + test('array rows support can be toggled', (t) => { + const db = new DatabaseSync(nextDb()); + t.after(() => { db.close(); }); + const setup = 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(query.get(), { __proto__: null, key: 1, val: 'one' }); + t.assert.strictEqual(query.setReturnArrays(true), undefined); + t.assert.deepStrictEqual(query.get(), [1, 'one']); + t.assert.strictEqual(query.setReturnArrays(false), undefined); + t.assert.deepStrictEqual(query.get(), { __proto__: null, key: 1, val: 'one' }); + + const allQuery = db.prepare('SELECT key, val FROM data ORDER BY key'); + t.assert.deepStrictEqual(allQuery.all(), [ + { __proto__: null, key: 1, val: 'one' }, + { __proto__: null, key: 2, val: 'two' }, + ]); + t.assert.strictEqual(allQuery.setReturnArrays(true), undefined); + t.assert.deepStrictEqual(allQuery.all(), [ + [1, 'one'], + [2, 'two'], + ]); + t.assert.strictEqual(allQuery.setReturnArrays(false), undefined); + t.assert.deepStrictEqual(allQuery.all(), [ + { __proto__: null, key: 1, val: 'one' }, + { __proto__: null, key: 2, val: 'two' }, + ]); + + const iterateQuery = db.prepare('SELECT key, val FROM data ORDER BY key'); + const objectRows = []; + for (const row of iterateQuery.iterate()) { + objectRows.push(row); + } + t.assert.deepStrictEqual(objectRows, [ + { __proto__: null, key: 1, val: 'one' }, + { __proto__: null, key: 2, val: 'two' }, + ]); + + t.assert.strictEqual(iterateQuery.setReturnArrays(true), undefined); + const arrayRows = []; + for (const row of iterateQuery.iterate()) { + arrayRows.push(row); + } + t.assert.deepStrictEqual(arrayRows, [ + [1, 'one'], + [2, 'two'], + ]); + }); + + test('throws when input is not a boolean', (t) => { + const db = new DatabaseSync(nextDb()); + t.after(() => { db.close(); }); + const setup = 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/, + }); + }); + + test('array rows with lots of columns', (t) => { + const db = new DatabaseSync(nextDb()); + t.after(() => { db.close(); }); + const setup = 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 row = query.get(); + t.assert.strictEqual(row.length, 10); + t.assert.strictEqual(row[0], 1); + t.assert.strictEqual(row[1], 'text1'); + t.assert.strictEqual(row[2], 1.1); + t.assert.ok(row[3] instanceof Uint8Array); + t.assert.strictEqual(row[4], 5); + t.assert.strictEqual(row[5], 'text2'); + t.assert.strictEqual(row[6], 2.2); + t.assert.ok(row[7] instanceof Uint8Array); + t.assert.strictEqual(row[8], 9); + t.assert.strictEqual(row[9], 'text3'); + }); + + test('array rows with BigInts', (t) => { + const db = new DatabaseSync(nextDb()); + t.after(() => { db.close(); }); + const setup = 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 = query.get(); + t.assert.strictEqual(row.length, 2); + t.assert.strictEqual(row[0], 1n); + t.assert.strictEqual(row[1], 9007199254740992n); + }); +}); + suite('StatementSync.prototype.setAllowBareNamedParameters()', () => { test('bare named parameter support can be toggled', (t) => { const db = new DatabaseSync(nextDb()); From e4f5ea3519b3d45c22029411dee969457f5630d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 01:58:10 +0100 Subject: [PATCH 02/15] fix linting --- src/node_sqlite.cc | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index c12c65a1462e17..71573d693ad451 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1689,7 +1689,7 @@ void StatementSync::IterateNextCallback( Local return_arrays_val; bool return_arrays = false; if (self->Get(context, FIXED_ONE_BYTE_STRING(isolate, "return_arrays")) - .ToLocal(&return_arrays_val) && + .ToLocal(&return_arrays_val) && return_arrays_val->IsBoolean()) { return_arrays = return_arrays_val.As()->Value(); } @@ -1800,15 +1800,14 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { } // Add the return_arrays flag to the iterator - auto return_arrays_pd = - v8::PropertyDescriptor(v8::Boolean::New( - isolate, stmt->return_arrays_), false); + auto return_arrays_pd = v8::PropertyDescriptor( + v8::Boolean::New(isolate, stmt->return_arrays_), false); return_arrays_pd.set_enumerable(false); return_arrays_pd.set_configurable(false); if (iterable_iterator - ->DefineProperty( - context, FIXED_ONE_BYTE_STRING( - isolate, "return_arrays"), return_arrays_pd) + ->DefineProperty(context, + FIXED_ONE_BYTE_STRING(isolate, "return_arrays"), + return_arrays_pd) .IsNothing()) { // An error will have been scheduled. return; @@ -1867,9 +1866,8 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { values.emplace_back(val); } - Local result = - Object::New( - isolate, Null(isolate), keys.data(), values.data(), num_cols); + Local result = Object::New( + isolate, Null(isolate), keys.data(), values.data(), num_cols); args.GetReturnValue().Set(result); } @@ -2144,7 +2142,7 @@ Local StatementSync::GetConstructorTemplate( SetProtoMethod( isolate, tmpl, "setReadBigInts", StatementSync::SetReadBigInts); SetProtoMethod( - isolate, tmpl, "setReturnArrays", StatementSync::SetReturnArrays); + isolate, tmpl, "setReturnArrays", StatementSync::SetReturnArrays); env->set_sqlite_statement_sync_constructor_template(tmpl); } return tmpl; From 6e5357d454aa0d75f418d3ac0717087403bc40ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 23:04:46 +0100 Subject: [PATCH 03/15] apply feedback --- src/env_properties.h | 1 + src/node_sqlite.cc | 31 ++++++++++++--------- test/parallel/test-sqlite-statement-sync.js | 29 +++++++++---------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/env_properties.h b/src/env_properties.h index 9e950c50791b11..b255216d7f44f4 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -335,6 +335,7 @@ V(require_string, "require") \ V(resource_string, "resource") \ V(retry_string, "retry") \ + V(return_arrays_string, "returnArrays") \ V(return_string, "return") \ V(salt_length_string, "saltLength") \ V(scheme_string, "scheme") \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 71573d693ad451..d20b88b5a92a19 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1641,8 +1641,7 @@ void StatementSync::IterateReturnCallback( args.GetReturnValue().Set(result); } -void StatementSync::IterateNextCallback( - const v8::FunctionCallbackInfo& args) { +void StatementSync::IterateNextCallback(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); auto isolate = env->isolate(); auto context = isolate->GetCurrentContext(); @@ -1688,7 +1687,7 @@ void StatementSync::IterateNextCallback( // Get the return_arrays flag from the iterator object Local return_arrays_val; bool return_arrays = false; - if (self->Get(context, FIXED_ONE_BYTE_STRING(isolate, "return_arrays")) + if (self->Get(context, env->return_arrays_string()) .ToLocal(&return_arrays_val) && return_arrays_val->IsBoolean()) { return_arrays = return_arrays_val.As()->Value(); @@ -1708,24 +1707,26 @@ void StatementSync::IterateNextCallback( return; } - Local keys[] = {env->done_string(), env->value_string()}; - Local values[] = {Boolean::New(isolate, true), Null(isolate)}; - static_assert(arraysize(keys) == arraysize(values)); + 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[0], &values[0], arraysize(keys)); + isolate, Null(isolate), keys.data(), values.data(), keys.size()); args.GetReturnValue().Set(result); return; } Local row; if (return_arrays) { - Local array_row = Array::New(isolate, num_cols); + LocalVector row_values(isolate); + row_values.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { Local val; if (!stmt->ColumnToValue(i).ToLocal(&val)) return; - if (array_row->Set(env->context(), i, val).IsNothing()) return; + row_values.emplace_back(val); } - row = array_row; + row = Array::New(isolate, row_values.data(), row_values.size()); } else { LocalVector row_keys(isolate); row_keys.reserve(num_cols); @@ -1806,7 +1807,7 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { return_arrays_pd.set_configurable(false); if (iterable_iterator ->DefineProperty(context, - FIXED_ONE_BYTE_STRING(isolate, "return_arrays"), + env->return_arrays_string(), return_arrays_pd) .IsNothing()) { // An error will have been scheduled. @@ -1844,12 +1845,16 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { } if (stmt->return_arrays_) { - Local result = Array::New(isolate, num_cols); + LocalVector row_values(isolate); + row_values.reserve(num_cols); + for (int i = 0; i < num_cols; ++i) { Local val; if (!stmt->ColumnToValue(i).ToLocal(&val)) return; - if (result->Set(env->context(), i, val).IsNothing()) return; + row_values.emplace_back(val); } + + Local result = Array::New(isolate, row_values.data(), row_values.size()); args.GetReturnValue().Set(result); } else { LocalVector keys(isolate); diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index f13c51a81d22e5..155e5a1ef8b59d 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -415,6 +415,18 @@ suite('StatementSync.prototype.setReturnArrays()', () => { }); test('array rows with lots of columns', (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 DatabaseSync(nextDb()); t.after(() => { db.close(); }); const setup = db.exec(` @@ -433,20 +445,11 @@ suite('StatementSync.prototype.setReturnArrays()', () => { query.setReturnArrays(true); const row = query.get(); - t.assert.strictEqual(row.length, 10); - t.assert.strictEqual(row[0], 1); - t.assert.strictEqual(row[1], 'text1'); - t.assert.strictEqual(row[2], 1.1); - t.assert.ok(row[3] instanceof Uint8Array); - t.assert.strictEqual(row[4], 5); - t.assert.strictEqual(row[5], 'text2'); - t.assert.strictEqual(row[6], 2.2); - t.assert.ok(row[7] instanceof Uint8Array); - t.assert.strictEqual(row[8], 9); - t.assert.strictEqual(row[9], 'text3'); + t.assert.deepStrictEqual(row, expected); }); test('array rows with BigInts', (t) => { + const expected = [1n, 9007199254740992n]; const db = new DatabaseSync(nextDb()); t.after(() => { db.close(); }); const setup = db.exec(` @@ -461,9 +464,7 @@ suite('StatementSync.prototype.setReturnArrays()', () => { query.setReadBigInts(true); const row = query.get(); - t.assert.strictEqual(row.length, 2); - t.assert.strictEqual(row[0], 1n); - t.assert.strictEqual(row[1], 9007199254740992n); + t.assert.deepStrictEqual(row, expected); }); }); From 39cb87a278dc270416f5e3bb770eda62c6c1ab97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 23:12:29 +0100 Subject: [PATCH 04/15] format cpp --- src/node_sqlite.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index d20b88b5a92a19..fa1ade31626b1e 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1641,7 +1641,8 @@ void StatementSync::IterateReturnCallback( args.GetReturnValue().Set(result); } -void StatementSync::IterateNextCallback(const FunctionCallbackInfo& args) { +void StatementSync::IterateNextCallback( + const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); auto isolate = env->isolate(); auto context = isolate->GetCurrentContext(); @@ -1807,8 +1808,7 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { return_arrays_pd.set_configurable(false); if (iterable_iterator ->DefineProperty(context, - env->return_arrays_string(), - return_arrays_pd) + env->return_arrays_string(), return_arrays_pd) .IsNothing()) { // An error will have been scheduled. return; @@ -1847,14 +1847,15 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { if (stmt->return_arrays_) { LocalVector row_values(isolate); row_values.reserve(num_cols); - + for (int i = 0; i < num_cols; ++i) { Local val; if (!stmt->ColumnToValue(i).ToLocal(&val)) return; row_values.emplace_back(val); } - - Local result = Array::New(isolate, row_values.data(), row_values.size()); + + Local result = Array::New( + isolate, row_values.data(), row_values.size()); args.GetReturnValue().Set(result); } else { LocalVector keys(isolate); From acca8b1ef586c6d5ce9249d8bfa6eac2fb55e798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 23:18:47 +0100 Subject: [PATCH 05/15] format cpp --- src/node_sqlite.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index fa1ade31626b1e..c0585328c0056f 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1807,8 +1807,8 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { return_arrays_pd.set_enumerable(false); return_arrays_pd.set_configurable(false); if (iterable_iterator - ->DefineProperty(context, - env->return_arrays_string(), return_arrays_pd) + ->DefineProperty( + context, env->return_arrays_string(), return_arrays_pd) .IsNothing()) { // An error will have been scheduled. return; @@ -1854,8 +1854,8 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { row_values.emplace_back(val); } - Local result = Array::New( - isolate, row_values.data(), row_values.size()); + Local result = + Array::New(isolate, row_values.data(), row_values.size()); args.GetReturnValue().Set(result); } else { LocalVector keys(isolate); From c733e910d012f88b45f6caa1633e38002e63a6c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 23:23:07 +0100 Subject: [PATCH 06/15] apply feedback --- src/node_sqlite.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index c0585328c0056f..ba45df1810324b 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1689,9 +1689,8 @@ void StatementSync::IterateNextCallback( Local return_arrays_val; bool return_arrays = false; if (self->Get(context, env->return_arrays_string()) - .ToLocal(&return_arrays_val) && - return_arrays_val->IsBoolean()) { - return_arrays = return_arrays_val.As()->Value(); + .ToLocal(&return_arrays_val)) { + return_arrays = return_arrays_val->IsTrue(); } int r = sqlite3_step(stmt->statement_); From de48b9391e39bd2cbfbb59f1a1f005bb7befd154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 23:28:21 +0100 Subject: [PATCH 07/15] use localvector and then create array --- src/node_sqlite.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index ba45df1810324b..c8e6d7aa077289 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1571,12 +1571,14 @@ void StatementSync::All(const FunctionCallbackInfo& args) { while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) { if (stmt->return_arrays_) { - Local row = Array::New(isolate, num_cols); + LocalVector row_values(isolate); + row_values.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { Local val; if (!stmt->ColumnToValue(i).ToLocal(&val)) return; - if (row->Set(env->context(), i, val).IsNothing()) return; + row_values.emplace_back(val); } + Local row = Array::New(isolate, row_values.data(), row_values.size()); rows.emplace_back(row); } else { if (row_keys.size() == 0) { From 5824701a566b56e10246b757f45f4fc0ffc4a45f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 23:34:38 +0100 Subject: [PATCH 08/15] apply feedback --- src/node_sqlite.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index c8e6d7aa077289..d322b254462e3f 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1747,11 +1747,12 @@ void StatementSync::IterateNextCallback( isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); } - Local keys[] = {env->done_string(), env->value_string()}; - Local values[] = {Boolean::New(isolate, false), row}; - static_assert(arraysize(keys) == arraysize(values)); + 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[0], &values[0], arraysize(keys)); + isolate, Null(isolate), keys.data(), values.data(), keys.size()); args.GetReturnValue().Set(result); } From d053ea57a606dac91df85c02eaba388245bfda5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Wed, 19 Mar 2025 23:36:35 +0100 Subject: [PATCH 09/15] format cpp --- src/node_sqlite.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index d322b254462e3f..cebc3f2dbd0807 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1578,7 +1578,8 @@ void StatementSync::All(const FunctionCallbackInfo& args) { if (!stmt->ColumnToValue(i).ToLocal(&val)) return; row_values.emplace_back(val); } - Local row = Array::New(isolate, row_values.data(), row_values.size()); + Local row = + Array::New(isolate, row_values.data(), row_values.size()); rows.emplace_back(row); } else { if (row_keys.size() == 0) { From 4d042f0ab62dcc0c81020f00f6d4f8cd9f2072ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 4 Apr 2025 11:44:53 +0200 Subject: [PATCH 10/15] rebase --- src/node_sqlite.cc | 257 +++++++++------------------------------------ 1 file changed, 51 insertions(+), 206 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index cebc3f2dbd0807..06a97ef6fa22c0 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1368,6 +1368,7 @@ StatementSync::StatementSync(Environment* env, statement_ = stmt; // In the future, some of these options could be set at the database // connection level and inherited by statements to reduce boilerplate. + return_arrays_ = false; use_big_ints_ = false; allow_bare_named_params_ = true; allow_unknown_named_params_ = false; @@ -1571,16 +1572,16 @@ void StatementSync::All(const FunctionCallbackInfo& args) { while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) { if (stmt->return_arrays_) { - LocalVector row_values(isolate); - row_values.reserve(num_cols); + LocalVector array_values(isolate); + array_values.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { Local val; if (!stmt->ColumnToValue(i).ToLocal(&val)) return; - row_values.emplace_back(val); + array_values.emplace_back(val); } - Local row = - Array::New(isolate, row_values.data(), row_values.size()); - rows.emplace_back(row); + Local row_array = Array::New( + isolate, array_values.data(), array_values.size()); + rows.emplace_back(row_array); } else { if (row_keys.size() == 0) { row_keys.reserve(num_cols); @@ -1599,9 +1600,9 @@ void StatementSync::All(const FunctionCallbackInfo& args) { row_values.emplace_back(val); } - Local row = Object::New( + Local row_obj = Object::New( isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); - rows.emplace_back(row); + rows.emplace_back(row_obj); } } @@ -1609,163 +1610,16 @@ 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"); - - // Get the return_arrays flag from the iterator object - Local return_arrays_val; - bool return_arrays = false; - if (self->Get(context, env->return_arrays_string()) - .ToLocal(&return_arrays_val)) { - return_arrays = return_arrays_val->IsTrue(); - } - - 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; - } - - Local row; - if (return_arrays) { - LocalVector row_values(isolate); - row_values.reserve(num_cols); - for (int i = 0; i < num_cols; ++i) { - Local val; - if (!stmt->ColumnToValue(i).ToLocal(&val)) return; - row_values.emplace_back(val); - } - row = Array::New(isolate, row_values.data(), row_values.size()); - } else { - 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); - } - - 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()); + CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void()); if (!stmt->BindParams(args)) { return; @@ -1785,6 +1639,7 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { BaseObjectPtr iter = StatementSyncIterator::Create(env, BaseObjectPtr(stmt)); + if (iter->object() ->GetPrototype() .As() @@ -1793,31 +1648,7 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { return; } - auto is_finished_pd = - v8::PropertyDescriptor(v8::Boolean::New(isolate, false), true); - is_finished_pd.set_enumerable(false); - is_finished_pd.set_configurable(false); - if (iterable_iterator - ->DefineProperty(context, env->isfinished_string(), is_finished_pd) - .IsNothing()) { - // An error will have been scheduled. - return; - } - - // Add the return_arrays flag to the iterator - auto return_arrays_pd = v8::PropertyDescriptor( - v8::Boolean::New(isolate, stmt->return_arrays_), false); - return_arrays_pd.set_enumerable(false); - return_arrays_pd.set_configurable(false); - if (iterable_iterator - ->DefineProperty( - context, env->return_arrays_string(), return_arrays_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) { @@ -1848,17 +1679,15 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { } if (stmt->return_arrays_) { - LocalVector row_values(isolate); - row_values.reserve(num_cols); - + LocalVector array_values(isolate); + array_values.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { Local val; if (!stmt->ColumnToValue(i).ToLocal(&val)) return; - row_values.emplace_back(val); + array_values.emplace_back(val); } - - Local result = - Array::New(isolate, row_values.data(), row_values.size()); + Local result = Array::New( + isolate, array_values.data(), array_values.size()); args.GetReturnValue().Set(result); } else { LocalVector keys(isolate); @@ -1876,7 +1705,7 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { } Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), num_cols); + isolate, Null(isolate), keys.data(), values.data(), num_cols); args.GetReturnValue().Set(result); } @@ -2222,20 +2051,22 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { if (iter->done_) { LocalVector values(isolate, - {Boolean::New(isolate, true), Null(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; } + bool return_arrays = iter->stmt_->return_arrays_; + 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)}); + {Boolean::New(isolate, true), Null(isolate)}); Local result = Object::New( isolate, Null(isolate), keys.data(), values.data(), keys.size()); args.GetReturnValue().Set(result); @@ -2243,22 +2074,36 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { } 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_value; + + if (return_arrays) { + LocalVector array_values(isolate); + array_values.reserve(num_cols); + for (int i = 0; i < num_cols; ++i) { + Local val; + if (!iter->stmt_->ColumnToValue(i).ToLocal(&val)) return; + array_values.emplace_back(val); + } + row_value = Array::New(isolate, array_values.data(), array_values.size()); + } else { + 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); + } + + row_value = Object::New( + isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); } - Local row = Object::New( - isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); - LocalVector values(isolate, {Boolean::New(isolate, false), row}); + LocalVector values(isolate, {Boolean::New(isolate, false), row_value}); Local result = Object::New( isolate, Null(isolate), keys.data(), values.data(), keys.size()); args.GetReturnValue().Set(result); From bb18d7c801c4b15664265283aa376ec7ef3dcd1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 4 Apr 2025 11:52:14 +0200 Subject: [PATCH 11/15] lint --- src/node_sqlite.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 06a97ef6fa22c0..9385ad69d103b2 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1579,8 +1579,8 @@ void StatementSync::All(const FunctionCallbackInfo& args) { if (!stmt->ColumnToValue(i).ToLocal(&val)) return; array_values.emplace_back(val); } - Local row_array = Array::New( - isolate, array_values.data(), array_values.size()); + Local row_array = + Array::New(isolate, array_values.data(), array_values.size()); rows.emplace_back(row_array); } else { if (row_keys.size() == 0) { @@ -1686,8 +1686,8 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { if (!stmt->ColumnToValue(i).ToLocal(&val)) return; array_values.emplace_back(val); } - Local result = Array::New( - isolate, array_values.data(), array_values.size()); + Local result = + Array::New(isolate, array_values.data(), array_values.size()); args.GetReturnValue().Set(result); } else { LocalVector keys(isolate); @@ -1705,7 +1705,7 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { } Local result = Object::New( - isolate, Null(isolate), keys.data(), values.data(), num_cols); + isolate, Null(isolate), keys.data(), values.data(), num_cols); args.GetReturnValue().Set(result); } @@ -2051,7 +2051,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { if (iter->done_) { LocalVector values(isolate, - {Boolean::New(isolate, true), Null(isolate)}); + {Boolean::New(isolate, true), Null(isolate)}); Local result = Object::New( isolate, Null(isolate), keys.data(), values.data(), keys.size()); args.GetReturnValue().Set(result); @@ -2066,7 +2066,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void()); sqlite3_reset(iter->stmt_->statement_); LocalVector values(isolate, - {Boolean::New(isolate, true), Null(isolate)}); + {Boolean::New(isolate, true), Null(isolate)}); Local result = Object::New( isolate, Null(isolate), keys.data(), values.data(), keys.size()); args.GetReturnValue().Set(result); From 18f3e3cf0b8c7ef06365d7168b8491c5e727598d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 4 Apr 2025 12:02:57 +0200 Subject: [PATCH 12/15] simplify --- src/node_sqlite.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 9385ad69d103b2..4593f3611a8674 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -2058,8 +2058,6 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { return; } - bool return_arrays = iter->stmt_->return_arrays_; - int r = sqlite3_step(iter->stmt_->statement_); if (r != SQLITE_ROW) { CHECK_ERROR_OR_THROW( @@ -2076,7 +2074,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { int num_cols = sqlite3_column_count(iter->stmt_->statement_); Local row_value; - if (return_arrays) { + if (iter->stmt_->return_arrays_) { LocalVector array_values(isolate); array_values.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { From 784638ca65f3c932f9656040ee7268cb72d7f868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 4 Apr 2025 17:02:37 +0200 Subject: [PATCH 13/15] move if outside the while --- src/node_sqlite.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 4593f3611a8674..7a5823ecc21933 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1568,10 +1568,9 @@ void StatementSync::All(const FunctionCallbackInfo& args) { auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); }); int num_cols = sqlite3_column_count(stmt->statement_); LocalVector rows(isolate); - LocalVector row_keys(isolate); - while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) { - if (stmt->return_arrays_) { + if (stmt->return_arrays_) { + while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) { LocalVector array_values(isolate); array_values.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { @@ -1582,7 +1581,11 @@ void StatementSync::All(const FunctionCallbackInfo& args) { Local row_array = Array::New(isolate, array_values.data(), array_values.size()); rows.emplace_back(row_array); - } else { + } + } else { + LocalVector row_keys(isolate); + + while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) { if (row_keys.size() == 0) { row_keys.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { From 7a408609c1ef03bc190e8192826be8c169ac2083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 4 Apr 2025 17:13:52 +0200 Subject: [PATCH 14/15] seperate array tests --- test/parallel/test-sqlite-statement-sync.js | 185 +++++++++++++------- 1 file changed, 126 insertions(+), 59 deletions(-) diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index 155e5a1ef8b59d..e9169ffd27fba0 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -344,77 +344,93 @@ suite('StatementSync.prototype.setReadBigInts()', () => { }); suite('StatementSync.prototype.setReturnArrays()', () => { - test('array rows support can be toggled', (t) => { + test('throws when input is not a boolean', (t) => { + const db = new DatabaseSync(nextDb()); + t.after(() => { db.close(); }); + const setup = 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('StatementSync.prototype.get() with array output', () => { + test('returns array row when setReturnArrays is true', (t) => { const db = new DatabaseSync(nextDb()); t.after(() => { db.close(); }); const setup = 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(query.get(), { __proto__: null, key: 1, val: 'one' }); - t.assert.strictEqual(query.setReturnArrays(true), undefined); + + query.setReturnArrays(true); t.assert.deepStrictEqual(query.get(), [1, 'one']); - t.assert.strictEqual(query.setReturnArrays(false), undefined); + + query.setReturnArrays(false); t.assert.deepStrictEqual(query.get(), { __proto__: null, key: 1, val: 'one' }); + }); + + test('returns array rows with BigInts when both flags are set', (t) => { + const expected = [1n, 9007199254740992n]; + const db = new DatabaseSync(nextDb()); + t.after(() => { db.close(); }); + const setup = 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 allQuery = db.prepare('SELECT key, val FROM data ORDER BY key'); - t.assert.deepStrictEqual(allQuery.all(), [ + const row = query.get(); + t.assert.deepStrictEqual(row, expected); + }); +}); + +suite('StatementSync.prototype.all() with array output', () => { + test('returns array rows when setReturnArrays is true', (t) => { + const db = new DatabaseSync(nextDb()); + t.after(() => { db.close(); }); + const setup = 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(query.all(), [ { __proto__: null, key: 1, val: 'one' }, { __proto__: null, key: 2, val: 'two' }, ]); - t.assert.strictEqual(allQuery.setReturnArrays(true), undefined); - t.assert.deepStrictEqual(allQuery.all(), [ + + query.setReturnArrays(true); + t.assert.deepStrictEqual(query.all(), [ [1, 'one'], [2, 'two'], ]); - t.assert.strictEqual(allQuery.setReturnArrays(false), undefined); - t.assert.deepStrictEqual(allQuery.all(), [ - { __proto__: null, key: 1, val: 'one' }, - { __proto__: null, key: 2, val: 'two' }, - ]); - - const iterateQuery = db.prepare('SELECT key, val FROM data ORDER BY key'); - const objectRows = []; - for (const row of iterateQuery.iterate()) { - objectRows.push(row); - } - t.assert.deepStrictEqual(objectRows, [ + + query.setReturnArrays(false); + t.assert.deepStrictEqual(query.all(), [ { __proto__: null, key: 1, val: 'one' }, { __proto__: null, key: 2, val: 'two' }, ]); - - t.assert.strictEqual(iterateQuery.setReturnArrays(true), undefined); - const arrayRows = []; - for (const row of iterateQuery.iterate()) { - arrayRows.push(row); - } - t.assert.deepStrictEqual(arrayRows, [ - [1, 'one'], - [2, 'two'], - ]); - }); - - test('throws when input is not a boolean', (t) => { - const db = new DatabaseSync(nextDb()); - t.after(() => { db.close(); }); - const setup = 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/, - }); }); - - test('array rows with lots of columns', (t) => { + + test('handles array rows with many columns', (t) => { const expected = [ 1, 'text1', @@ -443,28 +459,79 @@ suite('StatementSync.prototype.setReturnArrays()', () => { const query = db.prepare('SELECT * FROM wide_table'); query.setReturnArrays(true); - - const row = query.get(); - t.assert.deepStrictEqual(row, expected); + + const results = query.all(); + t.assert.strictEqual(results.length, 1); + t.assert.deepStrictEqual(results[0], expected); }); +}); - test('array rows with BigInts', (t) => { - const expected = [1n, 9007199254740992n]; +suite('StatementSync.prototype.iterate() with array output', () => { + test('iterates array rows when setReturnArrays is true', (t) => { const db = new DatabaseSync(nextDb()); t.after(() => { db.close(); }); const setup = db.exec(` - CREATE TABLE big_data(id INTEGER, big_num INTEGER); - INSERT INTO big_data VALUES (1, 9007199254740992); + 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 id, big_num FROM big_data'); + 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); - query.setReadBigInts(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', (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 stmt = db.prepare('SELECT key, val FROM test'); + stmt.setReturnArrays(true); + + const iterator = stmt.iterate(); + const results = []; - const row = query.get(); - t.assert.deepStrictEqual(row, expected); + 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 }, + ); }); }); From 5420ed55b1f20870f10abcbd8991d13fae2e9b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrg=C3=BCn=20Day=C4=B1o=C4=9Flu?= Date: Fri, 4 Apr 2025 17:19:38 +0200 Subject: [PATCH 15/15] fix whitespace and lint js --- src/node_sqlite.cc | 2 +- test/parallel/test-sqlite-statement-sync.js | 22 ++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 7a5823ecc21933..c1f82270d81e53 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1584,7 +1584,7 @@ void StatementSync::All(const FunctionCallbackInfo& args) { } } else { LocalVector row_keys(isolate); - + while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) { if (row_keys.size() == 0) { row_keys.reserve(num_cols); diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index e9169ffd27fba0..235c50ecec9851 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -373,14 +373,14 @@ suite('StatementSync.prototype.get() with array output', () => { const query = db.prepare('SELECT key, val FROM data WHERE key = 1'); t.assert.deepStrictEqual(query.get(), { __proto__: null, key: 1, val: 'one' }); - + query.setReturnArrays(true); t.assert.deepStrictEqual(query.get(), [1, 'one']); - + query.setReturnArrays(false); t.assert.deepStrictEqual(query.get(), { __proto__: null, key: 1, val: 'one' }); }); - + test('returns array rows with BigInts when both flags are set', (t) => { const expected = [1n, 9007199254740992n]; const db = new DatabaseSync(nextDb()); @@ -416,20 +416,20 @@ suite('StatementSync.prototype.all() with array output', () => { { __proto__: null, key: 1, val: 'one' }, { __proto__: null, key: 2, val: 'two' }, ]); - + query.setReturnArrays(true); t.assert.deepStrictEqual(query.all(), [ [1, 'one'], [2, 'two'], ]); - + query.setReturnArrays(false); t.assert.deepStrictEqual(query.all(), [ { __proto__: null, key: 1, val: 'one' }, { __proto__: null, key: 2, val: 'two' }, ]); }); - + test('handles array rows with many columns', (t) => { const expected = [ 1, @@ -459,7 +459,7 @@ suite('StatementSync.prototype.all() with array output', () => { const query = db.prepare('SELECT * FROM wide_table'); query.setReturnArrays(true); - + const results = query.all(); t.assert.strictEqual(results.length, 1); t.assert.deepStrictEqual(results[0], expected); @@ -478,7 +478,7 @@ suite('StatementSync.prototype.iterate() with array output', () => { 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()) { @@ -499,14 +499,14 @@ suite('StatementSync.prototype.iterate() with array output', () => { [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', (t) => { const db = new DatabaseSync(':memory:'); db.exec(` @@ -516,7 +516,7 @@ suite('StatementSync.prototype.iterate() with array output', () => { `); const stmt = db.prepare('SELECT key, val FROM test'); stmt.setReturnArrays(true); - + const iterator = stmt.iterate(); const results = [];