Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlite: return results with null prototype #54350

Merged
merged 2 commits into from
Aug 25, 2024
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
88 changes: 45 additions & 43 deletions src/node_sqlite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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:
Expand All @@ -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));
Expand All @@ -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 {}
Expand All @@ -474,38 +470,40 @@ 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;
}

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) {
Expand All @@ -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;
Expand All @@ -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;
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/node_sqlite.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion test/parallel/test-sqlite-data-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,31 @@ 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,
text: 'foo',
buf: u8a,
});
t.assert.deepStrictEqual(query.get(2), {
__proto__: null,
key: 2,
int: null,
double: null,
text: null,
buf: null,
});
t.assert.deepStrictEqual(query.get(3), {
__proto__: null,
key: 3,
int: 8,
double: 2.718,
text: 'bar',
buf: new TextEncoder().encode('x☃y☃'),
});
t.assert.deepStrictEqual(query.get(4), {
__proto__: null,
key: 4,
int: 99,
double: 0xf,
Expand Down Expand Up @@ -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 changes: 2 additions & 2 deletions test/parallel/test-sqlite-database-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
]);
});

Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-sqlite-named-parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
);
});

Expand All @@ -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 },
);
});

Expand Down
21 changes: 15 additions & 6 deletions test/parallel/test-sqlite-statement-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
});

Expand Down Expand Up @@ -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' },
]);
});
});
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-sqlite-transactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ suite('manual transactions', () => {
);
t.assert.deepStrictEqual(
db.prepare('SELECT * FROM data').all(),
[{ key: 100 }],
[{ __proto__: null, key: 100 }],
);
});

Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-sqlite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }]
);
});

Expand All @@ -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' },
);
});
Loading