From db6b6b72482d14002e67ba3d164717cc5fa5f2e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Thu, 24 Sep 2026 20:51:30 -0300 Subject: [PATCH] sqlite: use row template for result rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rows returned by get(), all() and iterate() were created with Object::New() and a null prototype. That produces a dictionary-mode object with its own property dictionary for every row. Cache a DictionaryTemplate per statement, rebuilt when the statement is re-prepared, so rows get fast properties and share a map. Column names that DictionaryTemplate cannot represent keep using Object::New(): array indices, duplicates, and non-ASCII names, which it would intern as Latin-1. Assisted-by: Claude Code Signed-off-by: Guilherme Araújo --- .../sqlite/sqlite-prepare-select-read.js | 53 ++++++++ src/node_sqlite.cc | 121 ++++++++++++------ src/node_sqlite.h | 7 +- test/parallel/test-sqlite-row-shape.js | 100 +++++++++++++++ 4 files changed, 238 insertions(+), 43 deletions(-) create mode 100644 benchmark/sqlite/sqlite-prepare-select-read.js create mode 100644 test/parallel/test-sqlite-row-shape.js diff --git a/benchmark/sqlite/sqlite-prepare-select-read.js b/benchmark/sqlite/sqlite-prepare-select-read.js new file mode 100644 index 000000000000..1139833b8d6c --- /dev/null +++ b/benchmark/sqlite/sqlite-prepare-select-read.js @@ -0,0 +1,53 @@ +'use strict'; +const common = require('../common.js'); +const sqlite = require('node:sqlite'); +const assert = require('assert'); + +const bench = common.createBenchmark(main, { + n: [20], + rows: [100, 10000], + method: ['all', 'iterate'], + access: ['named', 'keyed'], +}); + +const keys = ['text_column', 'integer_column', 'real_column', 'other_column']; + +function readNamed(row) { + return row.text_column.length + row.integer_column + row.real_column + + row.other_column; +} + +function readKeyed(row) { + let sum = 0; + for (let k = 0; k < keys.length; k++) sum += row[keys[k]] === null ? 0 : 1; + return sum; +} + +function main(conf) { + const db = new sqlite.DatabaseSync(':memory:'); + db.exec( + 'CREATE TABLE foo (text_column TEXT, integer_column INTEGER, ' + + 'real_column REAL, blob_column BLOB, other_column INTEGER)', + ); + const insert = db.prepare('INSERT INTO foo VALUES (?, ?, ?, ?, ?)'); + db.exec('BEGIN'); + for (let i = 0; i < conf.rows; i++) { + insert.run(`text ${i}`, i, i / 3, Buffer.from('blob'), i * 2); + } + db.exec('COMMIT'); + + const stmt = db.prepare('SELECT * FROM foo'); + const read = conf.access === 'named' ? readNamed : readKeyed; + const n = conf.n * (1e5 / conf.rows); + let sink = 0; + + bench.start(); + for (let i = 0; i < n; i++) { + for (const row of conf.method === 'all' ? stmt.all() : stmt.iterate()) { + sink += read(row); + } + } + bench.end(n * conf.rows); + + assert.ok(sink > 0); +} diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 3234d9881b51..f54e9006076e 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -3759,6 +3759,7 @@ void Statement::Finalize() { } void Statement::InvalidateColumnNameCache() { + cached_row_template_.Reset(); cached_column_names_.clear(); cached_column_names_reprepare_count_ = -1; } @@ -3981,20 +3982,53 @@ MaybeLocal Statement::ColumnNameToName(const int column) { .As(); } -// Populates `keys` with cached column names, rebuilding the cache if the -// statement was re-prepared. -bool Statement::GetCachedColumnNames(LocalVector* keys) { - Isolate* isolate = env()->isolate(); +// Mirrors V8's String::AsArrayIndex(): canonical decimal below 2^32 - 1. +static bool IsArrayIndex(std::string_view name) { + uint32_t index; + const char* end = name.data() + name.size(); + auto [ptr, ec] = std::from_chars(name.data(), end, index); + return ec == std::errc() && ptr == end && + (name.size() == 1 || name[0] != '0') && + index != std::numeric_limits::max(); +} + +// DictionaryTemplate CHECKs that no key is an array index, interns keys as +// Latin-1 rather than UTF-8, and assumes keys are unique. +static bool CanUseRowTemplate(const std::vector& names) { + std::unordered_set seen; + for (std::string_view name : names) { + if (!simdutf::validate_ascii(name.data(), name.size()) || + IsArrayIndex(name) || !seen.insert(name).second) { + return false; + } + } + return true; +} +// Rebuilds the row shape if the statement was re-prepared. +bool Statement::UpdateColumnNameCache() { const int reprepare_count = sqlite3_stmt_status(statement_.get(), SQLITE_STMTSTATUS_REPREPARE, false); - if (reprepare_count != cached_column_names_reprepare_count_) { - cached_column_names_.clear(); - const int num_cols = sqlite3_column_count(statement_.get()); - if (num_cols == 0) { - cached_column_names_reprepare_count_ = reprepare_count; - return true; - } + if (reprepare_count == cached_column_names_reprepare_count_) return true; + + Isolate* isolate = env()->isolate(); + InvalidateColumnNameCache(); + const int num_cols = sqlite3_column_count(statement_.get()); + std::vector names; + names.reserve(num_cols); + for (int i = 0; i < num_cols; ++i) { + const char* name = sqlite3_column_name(statement_.get(), i); + // ColumnNameToName() below reports the error. + if (name == nullptr) break; + names.emplace_back(name); + } + + if (names.size() == static_cast(num_cols) && + CanUseRowTemplate(names)) { + cached_row_template_.Reset( + isolate, + DictionaryTemplate::New(isolate, {names.data(), names.size()})); + } else { cached_column_names_.reserve(num_cols); for (int i = 0; i < num_cols; ++i) { Local key; @@ -4002,16 +4036,32 @@ bool Statement::GetCachedColumnNames(LocalVector* keys) { InvalidateColumnNameCache(); return false; } - cached_column_names_.emplace_back(Global(isolate, key)); + cached_column_names_.emplace_back(isolate, key); } - cached_column_names_reprepare_count_ = reprepare_count; } + cached_column_names_reprepare_count_ = reprepare_count; + return true; +} - keys->reserve(cached_column_names_.size()); +MaybeLocal Statement::NewRowObject(LocalVector* values) { + Isolate* isolate = env()->isolate(); + if (!cached_row_template_.IsEmpty()) { + // NewInstance() may replace entries, so it needs a MaybeLocal copy. + MaybeStackBuffer, 16> maybe_values(values->size()); + for (size_t i = 0; i < values->size(); ++i) maybe_values[i] = (*values)[i]; + return NewDictionaryInstanceNullProto(env()->context(), + cached_row_template_.Get(isolate), + {maybe_values.out(), values->size()}); + } + + DCHECK_EQ(cached_column_names_.size(), values->size()); + LocalVector keys(isolate); + keys.reserve(cached_column_names_.size()); for (const auto& name : cached_column_names_) { - keys->emplace_back(name.Get(isolate)); + keys.emplace_back(name.Get(isolate)); } - return true; + return Object::New( + isolate, Null(isolate), keys.data(), values->data(), values->size()); } MaybeLocal StatementExecutionHelper::ColumnToValue(Environment* env, @@ -4056,12 +4106,15 @@ MaybeLocal StatementExecutionHelper::All(Environment* env, int num_cols = 0; LocalVector rows(isolate); LocalVector row_values(isolate); - LocalVector row_keys(isolate); SteppingStatementGuard stepping(db, stmt); while ((r = sqlite3_step(stmt)) == SQLITE_ROW) { if (num_cols == 0) { num_cols = sqlite3_column_count(stmt); + // A running statement is only re-prepared before its first row. + if (!return_arrays && !statement->UpdateColumnNameCache()) { + return MaybeLocal(); + } } if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values) @@ -4074,16 +4127,10 @@ MaybeLocal StatementExecutionHelper::All(Environment* env, Array::New(isolate, row_values.data(), row_values.size()); rows.emplace_back(row_array); } else { - if (row_keys.size() == 0) { - // Reuses the statement's internalized column names instead of - // re-interning them on every call. - if (!statement->GetCachedColumnNames(&row_keys)) { - return MaybeLocal(); - } + Local row_obj; + if (!statement->NewRowObject(&row_values).ToLocal(&row_obj)) { + return MaybeLocal(); } - DCHECK_EQ(row_keys.size(), row_values.size()); - Local row_obj = Object::New( - isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); rows.emplace_back(row_obj); } } @@ -4221,16 +4268,10 @@ MaybeLocal StatementExecutionHelper::Get(Environment* env, if (return_arrays) { result = Array::New(isolate, row_values.data(), row_values.size()); } else { - LocalVector keys(isolate); - // Reuses the statement's internalized column names instead of - // re-interning them on every call. - if (!statement->GetCachedColumnNames(&keys)) { + if (!statement->UpdateColumnNameCache() || + !statement->NewRowObject(&row_values).ToLocal(&result)) { return MaybeLocal(); } - - DCHECK_EQ(keys.size(), row_values.size()); - result = Object::New( - isolate, Null(isolate), keys.data(), row_values.data(), num_cols); } RESET_AND_CHECK(isolate, db, stmt, needs_reset, MaybeLocal()); @@ -5031,7 +5072,6 @@ void StatementIterator::Next(const FunctionCallbackInfo& args) { int num_cols = sqlite3_column_count(iter->stmt_->statement_.get()); Local row_value; - LocalVector row_keys(isolate); LocalVector row_values(isolate); if (ExtractRowValues(env, @@ -5046,13 +5086,10 @@ void StatementIterator::Next(const FunctionCallbackInfo& args) { if (iter->stmt_->return_arrays_) { row_value = Array::New(isolate, row_values.data(), row_values.size()); } else { - // Use cached internalized column names to avoid repeated V8 string - // creation and enable hidden class sharing across row objects. - if (!iter->stmt_->GetCachedColumnNames(&row_keys)) return; - - DCHECK_EQ(row_keys.size(), row_values.size()); - row_value = Object::New( - isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols); + if (!iter->stmt_->UpdateColumnNameCache() || + !iter->stmt_->NewRowObject(&row_values).ToLocal(&row_value)) { + return; + } } MaybeLocal values[] = {Boolean::New(isolate, false), row_value}; diff --git a/src/node_sqlite.h b/src/node_sqlite.h index fc2f6cc1518f..2966f3cead81 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -396,7 +396,8 @@ class Statement : public BaseObject { static void Dispose(const v8::FunctionCallbackInfo& args); v8::MaybeLocal ColumnToValue(const int column); v8::MaybeLocal ColumnNameToName(const int column); - bool GetCachedColumnNames(v8::LocalVector* keys); + // Requires a successful UpdateColumnNameCache() for the current execution. + v8::MaybeLocal NewRowObject(v8::LocalVector* values); void Finalize(); bool IsFinalized(); @@ -415,9 +416,13 @@ class Statement : public BaseObject { uint64_t reset_generation_ = 0; std::optional> bare_named_params_; inline int ResetStatement(); + // Rows are built from the template, or from the names when they cannot be + // template keys. At most one of the two is populated. + v8::Global cached_row_template_; std::vector> cached_column_names_; int cached_column_names_reprepare_count_ = -1; void InvalidateColumnNameCache(); + bool UpdateColumnNameCache(); bool BindParams(const v8::FunctionCallbackInfo& args); bool BindValue(const v8::Local& value, const int index); diff --git a/test/parallel/test-sqlite-row-shape.js b/test/parallel/test-sqlite-row-shape.js new file mode 100644 index 000000000000..1d7d19b70d07 --- /dev/null +++ b/test/parallel/test-sqlite-row-shape.js @@ -0,0 +1,100 @@ +// Flags: --allow-natives-syntax +'use strict'; +const { skipIfSQLiteMissing } = require('../common'); +skipIfSQLiteMissing(); +const { DatabaseSync } = require('node:sqlite'); +const { suite, test } = require('node:test'); + +const hasFastProperties = eval('(o) => %HasFastProperties(o)'); +const haveSameMap = eval('(a, b) => %HaveSameMap(a, b)'); + +function assertSharedFastShape(t, rows) { + t.assert.ok(rows.length >= 2); + for (const row of rows) { + t.assert.strictEqual(Object.getPrototypeOf(row), null); + t.assert.ok(hasFastProperties(row)); + t.assert.ok(haveSameMap(rows[0], row)); + } +} + +function createDatabase() { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (a INTEGER, b TEXT); ' + + "INSERT INTO t VALUES (1, 'x'), (2, 'y'), (3, 'z');"); + return db; +} + +suite('result rows share a fast map', () => { + test('all()', (t) => { + using db = createDatabase(); + using stmt = db.prepare('SELECT a, b FROM t'); + assertSharedFastShape(t, stmt.all()); + }); + + test('get()', (t) => { + using db = createDatabase(); + using stmt = db.prepare('SELECT a, b FROM t WHERE a = ?'); + assertSharedFastShape(t, [stmt.get(1), stmt.get(2)]); + }); + + test('iterate()', (t) => { + using db = createDatabase(); + using stmt = db.prepare('SELECT a, b FROM t'); + assertSharedFastShape(t, [...stmt.iterate()]); + }); +}); + +test('rows with differing value types keep their values', (t) => { + using db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE t (v); ' + + "INSERT INTO t VALUES (NULL), (1), (1.5), ('s'), (x'01');"); + using stmt = db.prepare('SELECT v FROM t ORDER BY rowid'); + const expected = [ + { __proto__: null, v: null }, + { __proto__: null, v: 1 }, + { __proto__: null, v: 1.5 }, + { __proto__: null, v: 's' }, + { __proto__: null, v: new Uint8Array([1]) }, + ]; + t.assert.deepStrictEqual(stmt.all(), expected); + t.assert.deepStrictEqual([...stmt.iterate()], expected); + stmt.setReadBigInts(true); + t.assert.deepStrictEqual(stmt.all()[1], { __proto__: null, v: 1n }); +}); + +test('column names that cannot be template keys', (t) => { + using db = new DatabaseSync(':memory:'); + const cases = [ + ['SELECT 1 AS "0"', { __proto__: null, 0: 1 }], + ['SELECT 1 AS "4294967294"', { __proto__: null, 4294967294: 1 }], + ['SELECT 1 AS a, 2 AS a', { __proto__: null, a: 2 }], + ['SELECT 1 AS "café"', { __proto__: null, café: 1 }], + ]; + for (const [sql, row] of cases) { + using stmt = db.prepare(sql); + t.assert.deepStrictEqual(stmt.get(), row); + t.assert.deepStrictEqual(stmt.all(), [row]); + t.assert.deepStrictEqual([...stmt.iterate()], [row]); + } +}); + +test('names that look numeric but are not array indices', (t) => { + using db = new DatabaseSync(':memory:'); + using stmt = db.prepare( + 'SELECT 1 AS "01", 2 AS "4294967295", 3 AS "-1", 4 AS "1.5"'); + const row = { + __proto__: null, ['01']: 1, 4294967295: 2, ['-1']: 3, ['1.5']: 4, + }; + t.assert.deepStrictEqual(stmt.get(), row); + t.assert.deepStrictEqual([...stmt.iterate()], [row]); +}); + +test('iterate() picks up columns added by a re-prepare', (t) => { + using db = new DatabaseSync(':memory:'); + db.exec("CREATE TABLE t (a); INSERT INTO t VALUES ('x');"); + using stmt = db.prepare('SELECT * FROM t'); + t.assert.deepStrictEqual([...stmt.iterate()], [{ __proto__: null, a: 'x' }]); + db.exec("ALTER TABLE t ADD COLUMN b DEFAULT 'y';"); + t.assert.deepStrictEqual([...stmt.iterate()], + [{ __proto__: null, a: 'x', b: 'y' }]); +});