diff --git a/src/better_sqlite3.cpp b/src/better_sqlite3.cpp index a5ea755..67c402c 100644 --- a/src/better_sqlite3.cpp +++ b/src/better_sqlite3.cpp @@ -25,6 +25,7 @@ class Backup; #include "util/data-converter.cpp" #include "util/data.cpp" #if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 +#include "util/column-keys.cpp" #include "util/row-builder.cpp" #endif diff --git a/src/objects/statement-iterator.cpp b/src/objects/statement-iterator.cpp index 9c05fa4..ff69021 100644 --- a/src/objects/statement-iterator.cpp +++ b/src/objects/statement-iterator.cpp @@ -40,7 +40,7 @@ void StatementIterator::Next(NODE_ARGUMENTS info) { UseIsolate; UseContext; info.GetReturnValue().Set( - NewRecord(isolate, ctx, Data::GetRowJS(isolate, ctx, handle, safe_ints, mode), db_state->addon, false) + NewRecord(isolate, ctx, stmt->GetRowJS(isolate, ctx, safe_ints, mode), db_state->addon, false) ); } else { if (status == SQLITE_DONE) Return(info); diff --git a/src/objects/statement.cpp b/src/objects/statement.cpp index 21dfc9a..860c187 100644 --- a/src/objects/statement.cpp +++ b/src/objects/statement.cpp @@ -52,6 +52,21 @@ BindMap* Statement::GetBindMap(v8::Isolate* isolate) { Statement::Extras::Extras(sqlite3_uint64 id) : bind_map(0), id(id) {} +#if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 +RowBuilder Statement::NewRowBuilder(v8::Isolate* isolate, bool safe_ints) { + return RowBuilder(isolate, handle, safe_ints, &extras->column_keys); +} +#endif + +v8::Local Statement::GetRowJS(v8::Isolate* isolate, v8::Local ctx, bool safe_ints, char mode) { +#if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 + if (mode == Data::FLAT) { + return NewRowBuilder(isolate, safe_ints).GetRowJS(); + } +#endif + return Data::GetRowJS(isolate, ctx, handle, safe_ints, mode); +} + INIT(Statement::Init) { v8::Local t = NewConstructorTemplate(isolate, data, JS_new, "Statement"); SetPrototypeMethod(isolate, data, t, "run", JS_run); @@ -169,7 +184,7 @@ NODE_METHOD(Statement::JS_get) { STATEMENT_START(REQUIRE_STATEMENT_RETURNS_DATA, DOES_NOT_MUTATE); int status = sqlite3_step(handle); if (status == SQLITE_ROW) { - v8::Local result = Data::GetRowJS(isolate, OnlyContext, handle, stmt->safe_ints, stmt->mode); + v8::Local result = stmt->GetRowJS(isolate, OnlyContext, stmt->safe_ints, stmt->mode); sqlite3_reset(handle); STATEMENT_RETURN(result); } else if (status == SQLITE_DONE) { @@ -206,7 +221,7 @@ NODE_METHOD(Statement::JS_all) { rows.reserve(8); if (mode == Data::FLAT) { - RowBuilder rowBuilder(isolate, handle, safe_ints); + RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, safe_ints); while (sqlite3_step(handle) == SQLITE_ROW) { rows.emplace_back(rowBuilder.GetRowJS()); } @@ -285,7 +300,7 @@ NODE_METHOD(Statement::JS_explainQueryPlan) { rows.reserve(8); if (mode == Data::FLAT) { - RowBuilder rowBuilder(isolate, handle, safe_ints); + RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, safe_ints); while (sqlite3_step(handle) == SQLITE_ROW) { rows.emplace_back(rowBuilder.GetRowJS()); } diff --git a/src/objects/statement.hpp b/src/objects/statement.hpp index d387ee0..db6417a 100644 --- a/src/objects/statement.hpp +++ b/src/objects/statement.hpp @@ -14,6 +14,16 @@ class Statement : public node::ObjectWrap { friend class StatementIterator; // Returns the Statement's bind map (creates it upon first execution). BindMap* GetBindMap(v8::Isolate* isolate); +#if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 + // Returns a builder of object rows that reuses the Statement's cached + // column names. + RowBuilder NewRowBuilder(v8::Isolate* isolate, bool safe_ints); +#endif + + // Converts the current row to JavaScript, reusing the cached column names + // for object rows. + v8::Local GetRowJS(v8::Isolate* isolate, v8::Local ctx, bool safe_ints, char mode); + static INIT(Init); private: @@ -23,6 +33,9 @@ class Statement : public node::ObjectWrap { friend class StatementIterator; explicit Extras(sqlite3_uint64 id); BindMap bind_map; const sqlite3_uint64 id; +#if defined(NODE_MODULE_VERSION) && NODE_MODULE_VERSION >= 127 + ColumnKeys column_keys; +#endif }; explicit Statement( diff --git a/src/util/column-keys.cpp b/src/util/column-keys.cpp new file mode 100644 index 0000000..bc4907d --- /dev/null +++ b/src/util/column-keys.cpp @@ -0,0 +1,35 @@ +// The column names of a statement as internalized strings, cached across calls +// so that building object rows doesn't re-internalize them for every row. +// Names only change when SQLite re-prepares the statement (e.g. after a schema +// change behind `SELECT *`), so the cache is refreshed when the re-prepare +// count or the column count changes. +class ColumnKeys { +public: + + explicit ColumnKeys() : reprepares(-1) {} + + // Must be called after sqlite3_step(), which is where re-preparing happens. + void Get(v8::Isolate* isolate, sqlite3_stmt* handle, v8::LocalVector& out) { + const int count = sqlite3_column_count(handle); + const int current = sqlite3_stmt_status(handle, SQLITE_STMTSTATUS_REPREPARE, 0); + if (current != reprepares || static_cast(keys.size()) != count) { + keys.clear(); + keys.reserve(count); + for (int i = 0; i < count; ++i) { + keys.emplace_back(isolate, + InternalizedFromUtf8(isolate, sqlite3_column_name(handle, i), -1).As() + ); + } + reprepares = current; + } + out.clear(); + out.reserve(count); + for (const v8::Global& key : keys) { + out.emplace_back(key.Get(isolate)); + } + } + +private: + std::vector> keys; + int reprepares; +}; diff --git a/src/util/row-builder.cpp b/src/util/row-builder.cpp index 273f8e8..99884c4 100644 --- a/src/util/row-builder.cpp +++ b/src/util/row-builder.cpp @@ -4,24 +4,20 @@ class RowBuilder { explicit RowBuilder( v8::Isolate* isolate, sqlite3_stmt* handle, - bool safe_ints + bool safe_ints, + ColumnKeys* column_keys ) : isolate(isolate), handle(handle), column_count(-1), safe_ints(safe_ints), + column_keys(column_keys), keys(isolate) {} v8::Local GetRowJS() { if (column_count < 0) { - column_count = sqlite3_column_count(handle); - keys.reserve(column_count); - for (int i = 0; i < column_count; ++i) { - keys.emplace_back( - InternalizedFromUtf8(isolate, sqlite3_column_name(handle, i), -1) - .As() - ); - } + column_keys->Get(isolate, handle, keys); + column_count = static_cast(keys.size()); } v8::LocalVector values(isolate); @@ -45,5 +41,6 @@ class RowBuilder { sqlite3_stmt* handle; int column_count; const bool safe_ints; + ColumnKeys* const column_keys; v8::LocalVector keys; }; diff --git a/test/21.statement.get.js b/test/21.statement.get.js index bd7211f..90258f6 100644 --- a/test/21.statement.get.js +++ b/test/21.statement.get.js @@ -106,4 +106,18 @@ describe('Statement#get()', function () { this.db.prepare(SQL2).get({}) ).to.throw(RangeError); }); + it('should use the new column names after the schema changes', function () { + const stmt = this.db.prepare('SELECT * FROM entries ORDER BY rowid'); + // Several calls, so later ones reuse the cached column names. + const get = () => { + const rows = Array.from({ length: 100 }, () => stmt.get()); + for (const row of rows) expect(row).to.deep.equal(rows[0]); + return rows[0]; + }; + expect(get()).to.deep.equal({ a: 'foo', b: 1, c: 3.14, d: Buffer.alloc(4).fill(0xdd), e: null }); + this.db.prepare('ALTER TABLE entries RENAME COLUMN a TO z').run(); + expect(get()).to.deep.equal({ z: 'foo', b: 1, c: 3.14, d: Buffer.alloc(4).fill(0xdd), e: null }); + this.db.prepare('ALTER TABLE entries ADD COLUMN f INTEGER').run(); + expect(get()).to.deep.equal({ z: 'foo', b: 1, c: 3.14, d: Buffer.alloc(4).fill(0xdd), e: null, f: null }); + }); }); diff --git a/test/22.statement.all.js b/test/22.statement.all.js index d45987b..0a05cdf 100644 --- a/test/22.statement.all.js +++ b/test/22.statement.all.js @@ -126,4 +126,23 @@ describe('Statement#all()', function () { this.db.prepare(SQL2).all({}) ).to.throw(RangeError); }); + it('should use the new column names after the schema changes', function () { + // Enough rows that the change also shows up past the first ones. + this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 10 FROM entries').run(); + this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 20 FROM entries').run(); + this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 40 FROM entries').run(); + const stmt = this.db.prepare('SELECT * FROM entries ORDER BY rowid'); + const keys = () => { + const rows = stmt.all(); + expect(rows).to.have.lengthOf(80); + const result = Object.keys(rows[0]); + for (const row of rows) expect(Object.keys(row)).to.deep.equal(result); + return result; + }; + expect(keys()).to.deep.equal(['a', 'b', 'c', 'd', 'e']); + this.db.prepare('ALTER TABLE entries RENAME COLUMN a TO z').run(); + expect(keys()).to.deep.equal(['z', 'b', 'c', 'd', 'e']); + this.db.prepare('ALTER TABLE entries ADD COLUMN f INTEGER').run(); + expect(keys()).to.deep.equal(['z', 'b', 'c', 'd', 'e', 'f']); + }); }); diff --git a/test/23.statement.iterate.js b/test/23.statement.iterate.js index 8fe5a94..983b884 100644 --- a/test/23.statement.iterate.js +++ b/test/23.statement.iterate.js @@ -228,4 +228,23 @@ describe('Statement#iterate()', function () { this.db.prepare(SQL1).iterate('foo', 1, new (function(){})(), Buffer.alloc(4).fill(0xdd), null) ).to.throw(TypeError); }); + it('should use the new column names after the schema changes', function () { + // Enough rows that the change also shows up past the first ones. + this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 10 FROM entries').run(); + this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 20 FROM entries').run(); + this.db.prepare('INSERT INTO entries (a, b) SELECT a, b + 40 FROM entries').run(); + const stmt = this.db.prepare('SELECT * FROM entries ORDER BY rowid'); + const keys = () => { + const rows = [...stmt.iterate()]; + expect(rows).to.have.lengthOf(80); + const result = Object.keys(rows[0]); + for (const row of rows) expect(Object.keys(row)).to.deep.equal(result); + return result; + }; + expect(keys()).to.deep.equal(['a', 'b', 'c', 'd', 'e']); + this.db.prepare('ALTER TABLE entries RENAME COLUMN a TO z').run(); + expect(keys()).to.deep.equal(['z', 'b', 'c', 'd', 'e']); + this.db.prepare('ALTER TABLE entries ADD COLUMN f INTEGER').run(); + expect(keys()).to.deep.equal(['z', 'b', 'c', 'd', 'e', 'f']); + }); });