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
6 changes: 6 additions & 0 deletions doc/api/sqlite.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ added: v22.5.0
* `open` {boolean} If `true`, the database is opened by the constructor. When
this value is `false`, the database must be opened via the `open()` method.
**Default:** `true`.
* `enableForeignKeyConstraints` {boolean} If `true`, foreign key constraints
are enabled. This is recommended but can be disabled for compatibility with
legacy database schemas. The enforcement of foreign key constraints can be
enabled and disabled after opening the database using
[`PRAGMA foreign_keys`][]. **Default:** `true`.

Constructs a new `DatabaseSync` instance.

Expand Down Expand Up @@ -317,6 +322,7 @@ exception.

[SQL injection]: https://en.wikipedia.org/wiki/SQL_injection
[`--experimental-sqlite`]: cli.md#--experimental-sqlite
[`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys
[`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html
[`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html
[`sqlite3_exec()`]: https://www.sqlite.org/c3ref/exec.html
Expand Down
35 changes: 33 additions & 2 deletions src/node_sqlite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,14 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, sqlite3* db) {
DatabaseSync::DatabaseSync(Environment* env,
Local<Object> object,
Local<String> location,
bool open)
bool open,
bool enable_foreign_keys_on_open)
: BaseObject(env, object) {
MakeWeak();
node::Utf8Value utf8_location(env->isolate(), location);
location_ = utf8_location.ToString();
connection_ = nullptr;
enable_foreign_keys_on_open_ = enable_foreign_keys_on_open;

if (open) {
Open();
Expand Down Expand Up @@ -116,6 +118,15 @@ bool DatabaseSync::Open() {
int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, nullptr);
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);

int foreign_keys_enabled;
r = sqlite3_db_config(connection_,
SQLITE_DBCONFIG_ENABLE_FKEY,
static_cast<int>(enable_foreign_keys_on_open_),
&foreign_keys_enabled);
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
CHECK_EQ(foreign_keys_enabled, enable_foreign_keys_on_open_);

return true;
}

Expand Down Expand Up @@ -157,6 +168,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
}

bool open = true;
bool enable_foreign_keys = true;

if (args.Length() > 1) {
if (!args[1]->IsObject()) {
Expand All @@ -179,9 +191,28 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
}
open = open_v.As<Boolean>()->Value();
}

Local<String> enable_foreign_keys_string =
FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints");
Local<Value> enable_foreign_keys_v;
if (!options->Get(env->context(), enable_foreign_keys_string)
.ToLocal(&enable_foreign_keys_v)) {
return;
}
if (!enable_foreign_keys_v->IsUndefined()) {
if (!enable_foreign_keys_v->IsBoolean()) {
node::THROW_ERR_INVALID_ARG_TYPE(
env->isolate(),
"The \"options.enableForeignKeyConstraints\" argument must be a "
"boolean.");
return;
}
enable_foreign_keys = enable_foreign_keys_v.As<Boolean>()->Value();
}
}

new DatabaseSync(env, args.This(), args[0].As<String>(), open);
new DatabaseSync(
env, args.This(), args[0].As<String>(), open, enable_foreign_keys);
}

void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {
Expand Down
4 changes: 3 additions & 1 deletion src/node_sqlite.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class DatabaseSync : public BaseObject {
DatabaseSync(Environment* env,
v8::Local<v8::Object> object,
v8::Local<v8::String> location,
bool open);
bool open,
bool enable_foreign_keys_on_open);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking nit: totally not a fan of bool arguments like this. Would much prefer having either an options struct or enums for this purpose.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell I agree, I am planning to add at least one more option and to then wrap the *_on_open options in a struct.

void MemoryInfo(MemoryTracker* tracker) const override;
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand All @@ -43,6 +44,7 @@ class DatabaseSync : public BaseObject {
std::string location_;
sqlite3* connection_;
std::unordered_set<StatementSync*> statements_;
bool enable_foreign_keys_on_open_;
};

class StatementSync : public BaseObject {
Expand Down
36 changes: 36 additions & 0 deletions test/parallel/test-sqlite-database-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,42 @@ suite('DatabaseSync() constructor', () => {
message: /The "options\.open" argument must be a boolean/,
});
});

test('throws if options.enableForeignKeyConstraints is provided but is not a boolean', (t) => {
t.assert.throws(() => {
new DatabaseSync('foo', { enableForeignKeyConstraints: 5 });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: /The "options\.enableForeignKeyConstraints" argument must be a boolean/,
});
});

test('enables foreign key constraints by default', (t) => {
const dbPath = nextDb();
const db = new DatabaseSync(dbPath);
db.exec(`
CREATE TABLE foo (id INTEGER PRIMARY KEY);
CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id));
`);
t.after(() => { db.close(); });
t.assert.throws(() => {
db.exec('INSERT INTO bar (foo_id) VALUES (1)');
}, {
code: 'ERR_SQLITE_ERROR',
message: 'FOREIGN KEY constraint failed',
});
});

test('allows disabling foreign key constraints', (t) => {
const dbPath = nextDb();
const db = new DatabaseSync(dbPath, { enableForeignKeyConstraints: false });
db.exec(`
CREATE TABLE foo (id INTEGER PRIMARY KEY);
CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id));
`);
t.after(() => { db.close(); });
db.exec('INSERT INTO bar (foo_id) VALUES (1)');
});
});

suite('DatabaseSync.prototype.open()', () => {
Expand Down