Repository: nodejs/node
Pull request: sqlite: add setReturnArrays method to StatementSync

Add a new StatementSync method setReturnArrays() that allows query results to be returned as arrays instead of objects. This is more efficient when column names are meaningless, auto-generated, or when working with hundreds of columns.

Fixes https://github.com/nodejs/node/issues/57534

It's my first significant C++ PR, hopefully first of many :)

Changed files:
filename: src/env_properties.h
patch: @@ -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")                                                   \

filename: src/node_sqlite.cc
patch: @@ -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;
@@ -1567,28 +1568,45 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
   auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
   int num_cols = sqlite3_column_count(stmt->statement_);
   LocalVector<Value> rows(isolate);
-  LocalVector<Name> 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_) {
+    while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) {
+      LocalVector<Value> array_values(isolate);
+      array_values.reserve(num_cols);
       for (int i = 0; i < num_cols; ++i) {
-        Local<Name> key;
-        if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
-        row_keys.emplace_back(key);
+        Local<Value> val;
+        if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
+        array_values.emplace_back(val);
       }
+      Local<Array> row_array =
+          Array::New(isolate, array_values.data(), array_values.size());
+      rows.emplace_back(row_array);
     }
+  } else {
+    LocalVector<Name> 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) {
+          Local<Name> key;
+          if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
+          row_keys.emplace_back(key);
+        }
+      }
 
-    LocalVector<Value> row_values(isolate);
-    row_values.reserve(num_cols);
-    for (int i = 0; i < num_cols; ++i) {
-      Local<Value> val;
-      if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
-      row_values.emplace_back(val);
-    }
+      LocalVector<Value> row_values(isolate);
+      row_values.reserve(num_cols);
+      for (int i = 0; i < num_cols; ++i) {
+        Local<Value> val;
+        if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
+        row_values.emplace_back(val);
+      }
 
-    Local<Object> row = Object::New(
-        isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
-    rows.emplace_back(row);
+      Local<Object> row_obj = Object::New(
+          isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
+      rows.emplace_back(row_obj);
+    }
   }
 
   CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_DONE, void());
@@ -1601,9 +1619,10 @@ void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
   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;
@@ -1623,13 +1642,15 @@ void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
 
   BaseObjectPtr<StatementSyncIterator> iter =
       StatementSyncIterator::Create(env, BaseObjectPtr<StatementSync>(stmt));
+
   if (iter->object()
           ->GetPrototype()
           .As<Object>()
           ->SetPrototype(context, js_iterator_prototype)
           .IsNothing()) {
     return;
   }
+
   args.GetReturnValue().Set(iter->object());
 }
 
@@ -1660,24 +1681,37 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  LocalVector<Name> keys(isolate);
-  keys.reserve(num_cols);
-  LocalVector<Value> values(isolate);
-  values.reserve(num_cols);
+  if (stmt->return_arrays_) {
+    LocalVector<Value> array_values(isolate);
+    array_values.reserve(num_cols);
+    for (int i = 0; i < num_cols; ++i) {
+      Local<Value> val;
+      if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
+      array_values.emplace_back(val);
+    }
+    Local<Array> result =
+        Array::New(isolate, array_values.data(), array_values.size());
+    args.GetReturnValue().Set(result);
+  } else {
+    LocalVector<Name> keys(isolate);
+    keys.reserve(num_cols);
+    LocalVector<Value> values(isolate);
+    values.reserve(num_cols);
 
-  for (int i = 0; i < num_cols; ++i) {
-    Local<Name> key;
-    if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
-    Local<Value> 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<Name> key;
+      if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
+      Local<Value> val;
+      if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
+      keys.emplace_back(key);
+      values.emplace_back(val);
+    }
 
-  Local<Object> result =
-      Object::New(isolate, Null(isolate), keys.data(), values.data(), num_cols);
+    Local<Object> 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<Value>& args) {
@@ -1877,6 +1911,22 @@ void StatementSync::SetReadBigInts(const FunctionCallbackInfo<Value>& args) {
   stmt->use_big_ints_ = args[0]->IsTrue();
 }
 
+void StatementSync::SetReturnArrays(const FunctionCallbackInfo<Value>& 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<Value>& args) {
   THROW_ERR_ILLEGAL_CONSTRUCTOR(Environment::GetCurrent(args));
 }
@@ -1932,6 +1982,8 @@ Local<FunctionTemplate> 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;
@@ -2023,22 +2075,36 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
   }
 
   int num_cols = sqlite3_column_count(iter->stmt_->statement_);
-  LocalVector<Name> row_keys(isolate);
-  LocalVector<Value> row_values(isolate);
-  row_keys.reserve(num_cols);
-  row_values.reserve(num_cols);
-  for (int i = 0; i < num_cols; ++i) {
-    Local<Name> key;
-    if (!iter->stmt_->ColumnNameToName(i).ToLocal(&key)) return;
-    Local<Value> val;
-    if (!iter->stmt_->ColumnToValue(i).ToLocal(&val)) return;
-    row_keys.emplace_back(key);
-    row_values.emplace_back(val);
+  Local<Value> row_value;
+
+  if (iter->stmt_->return_arrays_) {
+    LocalVector<Value> array_values(isolate);
+    array_values.reserve(num_cols);
+    for (int i = 0; i < num_cols; ++i) {
+      Local<Value> 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<Name> row_keys(isolate);
+    LocalVector<Value> row_values(isolate);
+    row_keys.reserve(num_cols);
+    row_values.reserve(num_cols);
+    for (int i = 0; i < num_cols; ++i) {
+      Local<Name> key;
+      if (!iter->stmt_->ColumnNameToName(i).ToLocal(&key)) return;
+      Local<Value> 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<Object> row = Object::New(
-      isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
-  LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row});
+  LocalVector<Value> values(isolate, {Boolean::New(isolate, false), row_value});
   Local<Object> result = Object::New(
       isolate, Null(isolate), keys.data(), values.data(), keys.size());
   args.GetReturnValue().Set(result);

filename: src/node_sqlite.h
patch: @@ -126,6 +126,7 @@ class StatementSync : public BaseObject {
   static void SetAllowUnknownNamedParameters(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void SetReadBigInts(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void SetReturnArrays(const v8::FunctionCallbackInfo<v8::Value>& args);
   void Finalize();
   bool IsFinalized();
 
@@ -136,6 +137,7 @@ class StatementSync : public BaseObject {
   ~StatementSync() override;
   BaseObjectPtr<DatabaseSync> db_;
   sqlite3_stmt* statement_;
+  bool return_arrays_ = false;
   bool use_big_ints_;
   bool allow_bare_named_params_;
   bool allow_unknown_named_params_;

filename: test/parallel/test-sqlite-statement-sync.js
patch: @@ -343,6 +343,198 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
   });
 });
 
+suite('StatementSync.prototype.setReturnArrays()', () => {
+  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');
+    `);
+    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' });
+
+    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());
+    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.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' },
+    ]);
+
+    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,
+      '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(`
+      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 = query.all();
+    t.assert.strictEqual(results.length, 1);
+    t.assert.deepStrictEqual(results[0], expected);
+  });
+});
+
+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 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', (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 = [];
+
+    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('StatementSync.prototype.setAllowBareNamedParameters()', () => {
   test('bare named parameter support can be toggled', (t) => {
     const db = new DatabaseSync(nextDb());