Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/objects/statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ 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);
RowBuilder Statement::NewRowBuilder(v8::Isolate* isolate, v8::Local<v8::Context> ctx, bool safe_ints) {
return RowBuilder(isolate, ctx, handle, safe_ints, &extras->column_keys);
}
#endif

v8::Local<v8::Value> Statement::GetRowJS(v8::Isolate* isolate, v8::Local<v8::Context> 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();
return NewRowBuilder(isolate, ctx, safe_ints).GetRowJS();
}
#endif
return Data::GetRowJS(isolate, ctx, handle, safe_ints, mode);
Expand Down Expand Up @@ -221,7 +221,7 @@ NODE_METHOD(Statement::JS_all) {
rows.reserve(8);

if (mode == Data::FLAT) {
RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, safe_ints);
RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, ctx, safe_ints);
while (sqlite3_step(handle) == SQLITE_ROW) {
rows.emplace_back(rowBuilder.GetRowJS());
}
Expand Down Expand Up @@ -300,7 +300,7 @@ NODE_METHOD(Statement::JS_explainQueryPlan) {
rows.reserve(8);

if (mode == Data::FLAT) {
RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, safe_ints);
RowBuilder rowBuilder = stmt->NewRowBuilder(isolate, ctx, safe_ints);
while (sqlite3_step(handle) == SQLITE_ROW) {
rows.emplace_back(rowBuilder.GetRowJS());
}
Expand Down
4 changes: 2 additions & 2 deletions src/objects/statement.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class Statement : public node::ObjectWrap { friend class StatementIterator;

#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);
// column names and template.
RowBuilder NewRowBuilder(v8::Isolate* isolate, v8::Local<v8::Context> ctx, bool safe_ints);
#endif

// Converts the current row to JavaScript, reusing the cached column names
Expand Down
96 changes: 84 additions & 12 deletions src/util/column-keys.cpp
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
// 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.
// The column names of a statement, 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.
//
// When it can, it also caches a v8::DictionaryTemplate for the names. Objects
// made from it have fast properties and share a hidden class, while
// v8::Object::New() makes dictionary-mode objects, which are slower to create
// and to read. Creating the template costs about as much as building 10-20
// rows with v8::Object::New(), so it is only created once the statement has
// built kRowsBeforeTemplate object rows.
class ColumnKeys {
public:

static constexpr int kRowsBeforeTemplate = 16;

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<v8::Name>& out) {
const int count = sqlite3_column_count(handle);
const int current = sqlite3_stmt_status(handle, SQLITE_STMTSTATUS_REPREPARE, 0);
if (current != reprepares || static_cast<int>(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<v8::Name>()
);
}
Refresh(isolate, handle, count);
reprepares = current;
}
out.clear();
Expand All @@ -29,7 +31,77 @@ class ColumnKeys {
}
}

// Called for each object row built with the names last returned by Get().
// Returns the template for those names, creating it once enough rows have
// been built. Returns an empty handle before then, or if the names can't
// be used for a template.
v8::Local<v8::DictionaryTemplate> TemplateForRow(v8::Isolate* isolate) {
if (dictionary_template.IsEmpty() && !names.empty() && ++rows >= kRowsBeforeTemplate) {
Comment thread
arv marked this conversation as resolved.
Resolved
std::vector<std::string_view> views(names.begin(), names.end());
dictionary_template.Reset(isolate, v8::DictionaryTemplate::New(isolate,
v8::MemorySpan<const std::string_view>(views.data(), views.size())
));
names.clear();
}
return dictionary_template.Get(isolate);
}

private:

void Refresh(v8::Isolate* isolate, sqlite3_stmt* handle, int count) {
keys.clear();
keys.reserve(count);
names.clear();
names.reserve(count);
for (int i = 0; i < count; ++i) {
const char* name = sqlite3_column_name(handle, i);
keys.emplace_back(isolate, InternalizedFromUtf8(isolate, name, -1).As<v8::Name>());
names.emplace_back(name);
}
dictionary_template.Reset();
rows = 0;
if (!CanUseTemplate(names)) names.clear();
}

// v8::DictionaryTemplate reads names as Latin-1, so non-ASCII UTF-8 names
// would be garbled, and it crashes on duplicate names and on names that are
// array indices. With kMaxTemplateNames or more names its instances start
// from a dictionary map, which V8 doesn't support there (it DCHECKs), and
// they would be dictionary-mode objects anyway. Rows with such names are
// built with v8::Object::New().
static bool CanUseTemplate(const std::vector<std::string>& names) {
if (names.size() >= kMaxTemplateNames) return false;
std::set<std::string_view> seen;
for (const std::string& name : names) {
for (const char c : name) {
if (static_cast<unsigned char>(c) >= 0x80) return false;
}
if (IsArrayIndex(name) || !seen.insert(name).second) return false;
}
return true;
}

// Whether `name` is the canonical string of an integer in [0, 2^32 - 2].
static bool IsArrayIndex(const std::string& name) {
if (name.empty() || name.size() > 10) return false;
if (name.size() > 1 && name[0] == '0') return false;
uint64_t value = 0;
for (const char c : name) {
if (c < '0' || c > '9') return false;
value = value * 10 + static_cast<uint64_t>(c - '0');
}
return value <= 4294967294u;
}

// V8's JSObject::kMapCacheSize: object literals with this many properties
// get a dictionary map.
static constexpr size_t kMaxTemplateNames = 128;

std::vector<v8::Global<v8::Name>> keys;
// The names to create the template from; empty once it is created, or if
// they can't be used for one.
std::vector<std::string> names;
v8::Global<v8::DictionaryTemplate> dictionary_template;
int rows = 0;
int reprepares;
};
20 changes: 20 additions & 0 deletions src/util/row-builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ class RowBuilder {

explicit RowBuilder(
v8::Isolate* isolate,
v8::Local<v8::Context> ctx,
sqlite3_stmt* handle,
bool safe_ints,
ColumnKeys* column_keys
) :
isolate(isolate),
ctx(ctx),
handle(handle),
column_count(-1),
safe_ints(safe_ints),
Expand All @@ -19,6 +21,19 @@ class RowBuilder {
column_keys->Get(isolate, handle, keys);
column_count = static_cast<int>(keys.size());
}
if (dictionary_template.IsEmpty()) {
dictionary_template = column_keys->TemplateForRow(isolate);
}

if (!dictionary_template.IsEmpty()) {
template_values.resize(column_count);
for (int i = 0; i < column_count; ++i) {
template_values[i] = Data::GetValueJS(isolate, handle, i, safe_ints);
}
return dictionary_template->NewInstance(ctx,
v8::MemorySpan<v8::MaybeLocal<v8::Value>>(template_values.data(), template_values.size())
);
}

v8::LocalVector<v8::Value> values(isolate);
values.reserve(column_count);
Expand All @@ -38,9 +53,14 @@ class RowBuilder {

private:
v8::Isolate* isolate;
v8::Local<v8::Context> ctx;
sqlite3_stmt* handle;
int column_count;
const bool safe_ints;
ColumnKeys* const column_keys;
v8::LocalVector<v8::Name> keys;
v8::Local<v8::DictionaryTemplate> dictionary_template;
// Reused across rows. NewInstance() takes MaybeLocal values, for which
// there is no LocalVector.
std::vector<v8::MaybeLocal<v8::Value>> template_values;
};
34 changes: 33 additions & 1 deletion test/21.statement.get.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe('Statement#get()', function () {
});
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.
// Several calls, so rows are also built from the cached template.
const get = () => {
const rows = Array.from({ length: 100 }, () => stmt.get());
for (const row of rows) expect(row).to.deep.equal(rows[0]);
Expand All @@ -120,4 +120,36 @@ describe('Statement#get()', function () {
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 });
});
it('should build rows like plain objects for unusual column names', function () {
// Several calls, so rows are also built from the cached template.
const row = (sql) => {
const stmt = this.db.prepare(sql);
const results = Array.from({ length: 100 }, () => {
const result = stmt.get();
expect(Object.getPrototypeOf(result)).to.equal(Object.prototype);
return [Object.getOwnPropertyNames(result), Object.values(result)];
});
for (const result of results) expect(result).to.deep.equal(results[0]);
return results[0];
};
expect(row('SELECT 1 AS a, 2 AS b, 3 AS a')).to.deep.equal([['a', 'b'], [3, 2]]);
expect(row('SELECT 1 AS "1", 2 AS b, 3 AS "0"')).to.deep.equal([['0', '1', 'b'], [3, 1, 2]]);
expect(row('SELECT 1 AS "__proto__", 2 AS b')).to.deep.equal([['__proto__', 'b'], [1, 2]]);
expect(row('SELECT 1 AS "é", 2 AS "😀", 3 AS "", 4 AS "constructor"')).to.deep.equal([['é', '😀', '', 'constructor'], [1, 2, 3, 4]]);
expect(row('SELECT 1 AS "a""b", 2 AS "c\\d", 3 AS "e\nf"')).to.deep.equal([['a"b', 'c\\d', 'e\nf'], [1, 2, 3]]);
expect(row('SELECT 1 AS "4294967295", 2 AS "-1", 3 AS "01", 4 AS "1.5", 5 AS length')).to.deep.equal([['4294967295', '-1', '01', '1.5', 'length'], [1, 2, 3, 4, 5]]);
});
it('should build rows with many columns', function () {
for (const width of [127, 128, 300]) {
const names = Array.from({ length: width }, (_, i) => `c${i}`);
const stmt = this.db.prepare(`SELECT ${names.map((name, i) => `${i} AS ${name}`).join(', ')}`);
const expected = Object.fromEntries(names.map((name, i) => [name, i]));
// Several calls, so rows are also built from the cached template.
for (let i = 0; i < 100; ++i) {
const result = stmt.get();
expect(Object.keys(result)).to.deep.equal(names);
expect(result).to.deep.equal(expected);
}
}
});
});
2 changes: 1 addition & 1 deletion test/22.statement.all.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe('Statement#all()', function () {
).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.
// Many rows, so rows are also built from the cached template.
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();
Expand Down
2 changes: 1 addition & 1 deletion test/23.statement.iterate.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ describe('Statement#iterate()', function () {
).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.
// Many rows, so rows are also built from the cached template.
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();
Expand Down
Loading