Nodejs/node
1
0
Fork 0
mirror of https://github.com/nodejs/node.git synced 2024-11-21 10:59:27 +00:00
Code Issues Actions 25 Packages Projects Releases Wiki Activity

sqlite: return results with null prototype

Browse Source 
These objects are dictionaries, and a query can return columns with
special names like `__proto__` (which would be ignored without this
change).

Also construct the object by passing vectors of properties for better
performance and improve error handling by using `MaybeLocal`.

PR-URL: https://github.com/nodejs/node/pull/54350
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Michaël Zasso 2024-08-25 12:43:17 +02:00 committed by GitHub
parent c3fe2d60bd
commit 7fea0108d5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 76 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
src/node_sqlite.cc
View File

@ -25,6 +25,10 @@ using v8::FunctionTemplate;
using v8::Integer;
using v8::Isolate;
using v8::Local;
using v8::LocalVector;
using v8::MaybeLocal;
using v8::Name;
using v8::Null;
using v8::Number;
using v8::Object;
using v8::String;
@ -405,7 +409,7 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
return true;
}
Local<Value> StatementSync::ColumnToValue(const int column) {
MaybeLocal<Value> StatementSync::ColumnToValue(const int column) {
switch (sqlite3_column_type(statement_, column)) {
case SQLITE_INTEGER: {
sqlite3_int64 value = sqlite3_column_int64(statement_, column);
@ -419,7 +423,7 @@ Local<Value> StatementSync::ColumnToValue(const int column) {
"represented as a JavaScript number: %" PRId64,
column,
value);
return Local<Value>();
return MaybeLocal<Value>();
}
}
case SQLITE_FLOAT:
@ -428,14 +432,10 @@ Local<Value> StatementSync::ColumnToValue(const int column) {
case SQLITE_TEXT: {
const char* value = reinterpret_cast<const char*>(
sqlite3_column_text(statement_, column));
Local<Value> val;
if (!String::NewFromUtf8(env()->isolate(), value).ToLocal(&val)) {
return Local<Value>();
}
return val;
return String::NewFromUtf8(env()->isolate(), value).As<Value>();
}
case SQLITE_NULL:
return v8::Null(env()->isolate());
return Null(env()->isolate());
case SQLITE_BLOB: {
size_t size =
static_cast<size_t>(sqlite3_column_bytes(statement_, column));
@ -451,19 +451,15 @@ Local<Value> StatementSync::ColumnToValue(const int column) {
}
}
Local<Value> StatementSync::ColumnNameToValue(const int column) {
MaybeLocal<Name> StatementSync::ColumnNameToName(const int column) {
const char* col_name = sqlite3_column_name(statement_, column);
if (col_name == nullptr) {
node::THROW_ERR_INVALID_STATE(
env(), "Cannot get name of column %d", column);
return Local<Value>();
return MaybeLocal<Name>();
}
Local<String> key;
if (!String::NewFromUtf8(env()->isolate(), col_name).ToLocal(&key)) {
return Local<Value>();
}
return key;
return String::NewFromUtf8(env()->isolate(), col_name).As<Name>();
}
void StatementSync::MemoryInfo(MemoryTracker* tracker) const {}
@ -474,9 +470,9 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
Isolate* isolate = env->isolate();
int r = sqlite3_reset(stmt->statement_);
CHECK_ERROR_OR_THROW(
env->isolate(), stmt->db_->Connection(), r, SQLITE_OK, void());
CHECK_ERROR_OR_THROW(isolate, stmt->db_->Connection(), r, SQLITE_OK, void());
if (!stmt->BindParams(args)) {
return;
@ -484,28 +480,30 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
int num_cols = sqlite3_column_count(stmt->statement_);
std::vector<Local<Value>> rows;
LocalVector<Value> rows(isolate);
while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) {
Local<Object> row = Object::New(env->isolate());
LocalVector<Name> row_keys(isolate);
row_keys.reserve(num_cols);
LocalVector<Value> row_values(isolate);
row_values.reserve(num_cols);
for (int i = 0; i < num_cols; ++i) {
Local<Value> key = stmt->ColumnNameToValue(i);
if (key.IsEmpty()) return;
Local<Value> val = stmt->ColumnToValue(i);
if (val.IsEmpty()) return;
if (row->Set(env->context(), key, val).IsNothing()) {
return;
}
Local<Name> key;
if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
Local<Value> val;
if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
row_keys.emplace_back(key);
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);
}
CHECK_ERROR_OR_THROW(
env->isolate(), stmt->db_->Connection(), r, SQLITE_DONE, void());
args.GetReturnValue().Set(
Array::New(env->isolate(), rows.data(), rows.size()));
isolate, stmt->db_->Connection(), r, SQLITE_DONE, void());
args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size()));
}
void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
@ -514,9 +512,9 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");
Isolate* isolate = env->isolate();
int r = sqlite3_reset(stmt->statement_);
CHECK_ERROR_OR_THROW(
env->isolate(), stmt->db_->Connection(), r, SQLITE_OK, void());
CHECK_ERROR_OR_THROW(isolate, stmt->db_->Connection(), r, SQLITE_OK, void());
if (!stmt->BindParams(args)) {
return;
@ -526,7 +524,7 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
r = sqlite3_step(stmt->statement_);
if (r == SQLITE_DONE) return;
if (r != SQLITE_ROW) {
THROW_ERR_SQLITE_ERROR(env->isolate(), stmt->db_->Connection());
THROW_ERR_SQLITE_ERROR(isolate, stmt->db_->Connection());
return;
}
@ -535,19 +533,23 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
return;
}
Local<Object> result = Object::New(env->isolate());
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<Value> key = stmt->ColumnNameToValue(i);
if (key.IsEmpty()) return;
Local<Value> val = stmt->ColumnToValue(i);
if (val.IsEmpty()) return;
if (result->Set(env->context(), key, val).IsNothing()) {
return;
}
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);
args.GetReturnValue().Set(result);
}
@ -676,7 +678,7 @@ Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
if (tmpl.IsEmpty()) {
Isolate* isolate = env->isolate();
tmpl = NewFunctionTemplate(isolate, IllegalConstructor);
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "StatementSync"));
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "StatementSync"));
tmpl->InstanceTemplate()->SetInternalFieldCount(
StatementSync::kInternalFieldCount);
SetProtoMethod(isolate, tmpl, "all", StatementSync::All);

4
src/node_sqlite.h
View File

@ -80,8 +80,8 @@ class StatementSync : public BaseObject {
std::optional<std::map<std::string, std::string>> bare_named_params_;
bool BindParams(const v8::FunctionCallbackInfo<v8::Value>& args);
bool BindValue(const v8::Local<v8::Value>& value, const int index);
v8::Local<v8::Value> ColumnToValue(const int column);
v8::Local<v8::Value> ColumnNameToValue(const int column);
v8::MaybeLocal<v8::Value> ColumnToValue(const int column);
v8::MaybeLocal<v8::Name> ColumnNameToName(const int column);
};
} // namespace sqlite

6
test/parallel/test-sqlite-data-types.js
View File

@ -49,6 +49,7 @@ suite('data binding and mapping', () => {
const query = db.prepare('SELECT * FROM types WHERE key = ?');
t.assert.deepStrictEqual(query.get(1), {
__proto__: null,
key: 1,
int: 42,
double: 3.14159,
@ -56,6 +57,7 @@ suite('data binding and mapping', () => {
buf: u8a,
});
t.assert.deepStrictEqual(query.get(2), {
__proto__: null,
key: 2,
int: null,
double: null,
@ -63,6 +65,7 @@ suite('data binding and mapping', () => {
buf: null,
});
t.assert.deepStrictEqual(query.get(3), {
__proto__: null,
key: 3,
int: 8,
double: 2.718,
@ -70,6 +73,7 @@ suite('data binding and mapping', () => {
buf: new TextEncoder().encode('x☃y☃'),
});
t.assert.deepStrictEqual(query.get(4), {
__proto__: null,
key: 4,
int: 99,
double: 0xf,
@ -151,7 +155,7 @@ suite('data binding and mapping', () => {
);
t.assert.deepStrictEqual(
db.prepare('SELECT * FROM data ORDER BY key').all(),
[{ key: 1, val: 5 }, { key: 2, val: null }],
[{ __proto__: null, key: 1, val: 5 }, { __proto__: null, key: 2, val: null }],
);
});
});

4
test/parallel/test-sqlite-database-sync.js
View File

@ -143,8 +143,8 @@ suite('DatabaseSync.prototype.exec()', () => {
t.assert.strictEqual(result, undefined);
const stmt = db.prepare('SELECT * FROM data ORDER BY key');
t.assert.deepStrictEqual(stmt.all(), [
{ key: 1, val: 2 },
{ key: 8, val: 9 },
{ __proto__: null, key: 1, val: 2 },
{ __proto__: null, key: 8, val: 9 },
]);
});

4
test/parallel/test-sqlite-named-parameters.js
View File

@ -42,7 +42,7 @@ suite('named parameters', () => {
stmt.run({ k: 1, v: 9 });
t.assert.deepStrictEqual(
db.prepare('SELECT * FROM data').get(),
{ key: 1, val: 9 },
{ __proto__: null, key: 1, val: 9 },
);
});
@ -57,7 +57,7 @@ suite('named parameters', () => {
stmt.run({ k: 1 });
t.assert.deepStrictEqual(
db.prepare('SELECT * FROM data').get(),
{ key: 1, val: 1 },
{ __proto__: null, key: 1, val: 1 },
);
});

21
test/parallel/test-sqlite-statement-sync.js
View File

@ -43,7 +43,15 @@ suite('StatementSync.prototype.get()', () => {
t.assert.strictEqual(stmt.get('key1', 'val1'), undefined);
t.assert.strictEqual(stmt.get('key2', 'val2'), undefined);
stmt = db.prepare('SELECT * FROM storage ORDER BY key');
t.assert.deepStrictEqual(stmt.get(), { key: 'key1', val: 'val1' });
t.assert.deepStrictEqual(stmt.get(), { __proto__: null, key: 'key1', val: 'val1' });
});
test('executes a query that returns special columns', (t) => {
const db = new DatabaseSync(nextDb());
t.after(() => { db.close(); });
const stmt = db.prepare('SELECT 1 as __proto__, 2 as constructor, 3 as toString');
// eslint-disable-next-line no-dupe-keys
t.assert.deepStrictEqual(stmt.get(), { __proto__: null, ['__proto__']: 1, constructor: 2, toString: 3 });
});
});
@ -71,8 +79,8 @@ suite('StatementSync.prototype.all()', () => {
);
stmt = db.prepare('SELECT * FROM storage ORDER BY key');
t.assert.deepStrictEqual(stmt.all(), [
{ key: 'key1', val: 'val1' },
{ key: 'key2', val: 'val2' },
{ __proto__: null, key: 'key1', val: 'val1' },
{ __proto__: null, key: 'key2', val: 'val2' },
]);
});
});
@ -171,11 +179,11 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
t.assert.strictEqual(setup, undefined);
const query = db.prepare('SELECT val FROM data');
t.assert.deepStrictEqual(query.get(), { val: 42 });
t.assert.deepStrictEqual(query.get(), { __proto__: null, val: 42 });
t.assert.strictEqual(query.setReadBigInts(true), undefined);
t.assert.deepStrictEqual(query.get(), { val: 42n });
t.assert.deepStrictEqual(query.get(), { __proto__: null, val: 42n });
t.assert.strictEqual(query.setReadBigInts(false), undefined);
t.assert.deepStrictEqual(query.get(), { val: 42 });
t.assert.deepStrictEqual(query.get(), { __proto__: null, val: 42 });
const insert = db.prepare('INSERT INTO data (key) VALUES (?)');
t.assert.deepStrictEqual(
@ -223,6 +231,7 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
const good = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`);
good.setReadBigInts(true);
t.assert.deepStrictEqual(good.get(), {
__proto__: null,
[`${Number.MAX_SAFE_INTEGER} + 1`]: 2n ** 53n,
});
});

2
test/parallel/test-sqlite-transactions.js
View File

@ -37,7 +37,7 @@ suite('manual transactions', () => {
);
t.assert.deepStrictEqual(
db.prepare('SELECT * FROM data').all(),
[{ key: 100 }],
[{ __proto__: null, key: 100 }],
);
});

8
test/parallel/test-sqlite.js
View File

@ -77,11 +77,11 @@ test('in-memory databases are supported', (t) => {
t.assert.strictEqual(setup2, undefined);
t.assert.deepStrictEqual(
db1.prepare('SELECT * FROM data').all(),
[{ key: 1 }]
[{ __proto__: null, key: 1 }]
);
t.assert.deepStrictEqual(
db2.prepare('SELECT * FROM data').all(),
[{ key: 1 }]
[{ __proto__: null, key: 1 }]
);
});
@ -90,10 +90,10 @@ test('PRAGMAs are supported', (t) => {
t.after(() => { db.close(); });
t.assert.deepStrictEqual(
db.prepare('PRAGMA journal_mode = WAL').get(),
{ journal_mode: 'wal' },
{ __proto__: null, journal_mode: 'wal' },
);
t.assert.deepStrictEqual(
db.prepare('PRAGMA journal_mode').get(),
{ journal_mode: 'wal' },
{ __proto__: null, journal_mode: 'wal' },
);
});