Skip to content

Commit dd3a560

Browse files
committed
sqlite: enable foreign key constraints by default
For historical reasons and to maintain compatibibility with legacy database schemas, SQLite does not enable foreign key constraints by default. For new applications, however, this behavior is undesirable. Currently, any application that wishes to use foreign keys must use PRAGMA foreign_keys = ON; to explicitly enable enforcement of such constraints. This commit changes the behavior of the SQLite API built into Node.js to enable foreign key constraints by default. This behavior can be overridden by users to maintain compatibility with legacy database schemas. PR-URL: nodejs#54777 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 585a778 commit dd3a560

File tree

4 files changed

+78
-3
lines changed

4 files changed

+78
-3
lines changed

doc/api/sqlite.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ added: v22.5.0
107107
* `open` {boolean} If `true`, the database is opened by the constructor. When
108108
this value is `false`, the database must be opened via the `open()` method.
109109
**Default:** `true`.
110+
* `enableForeignKeyConstraints` {boolean} If `true`, foreign key constraints
111+
are enabled. This is recommended but can be disabled for compatibility with
112+
legacy database schemas. The enforcement of foreign key constraints can be
113+
enabled and disabled after opening the database using
114+
[`PRAGMA foreign_keys`][]. **Default:** `true`.
110115

111116
Constructs a new `DatabaseSync` instance.
112117

@@ -386,6 +391,7 @@ exception.
386391
[Changesets and Patchsets]: https://www.sqlite.org/sessionintro.html#changesets_and_patchsets
387392
[SQL injection]: https://en.wikipedia.org/wiki/SQL_injection
388393
[`--experimental-sqlite`]: cli.md#--experimental-sqlite
394+
[`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys
389395
[`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html
390396
[`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html
391397
[`sqlite3_exec()`]: https://www.sqlite.org/c3ref/exec.html

src/node_sqlite.cc

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,14 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) {
9393
DatabaseSync::DatabaseSync(Environment* env,
9494
Local<Object> object,
9595
Local<String> location,
96-
bool open)
96+
bool open,
97+
bool enable_foreign_keys_on_open)
9798
: BaseObject(env, object) {
9899
MakeWeak();
99100
node::Utf8Value utf8_location(env->isolate(), location);
100101
location_ = utf8_location.ToString();
101102
connection_ = nullptr;
103+
enable_foreign_keys_on_open_ = enable_foreign_keys_on_open;
102104

103105
if (open) {
104106
Open();
@@ -137,6 +139,15 @@ bool DatabaseSync::Open() {
137139
int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
138140
int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, nullptr);
139141
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
142+
143+
int foreign_keys_enabled;
144+
r = sqlite3_db_config(connection_,
145+
SQLITE_DBCONFIG_ENABLE_FKEY,
146+
static_cast<int>(enable_foreign_keys_on_open_),
147+
&foreign_keys_enabled);
148+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
149+
CHECK_EQ(foreign_keys_enabled, enable_foreign_keys_on_open_);
150+
140151
return true;
141152
}
142153

@@ -178,6 +189,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
178189
}
179190

180191
bool open = true;
192+
bool enable_foreign_keys = true;
181193

182194
if (args.Length() > 1) {
183195
if (!args[1]->IsObject()) {
@@ -200,9 +212,28 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
200212
}
201213
open = open_v.As<Boolean>()->Value();
202214
}
215+
216+
Local<String> enable_foreign_keys_string =
217+
FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints");
218+
Local<Value> enable_foreign_keys_v;
219+
if (!options->Get(env->context(), enable_foreign_keys_string)
220+
.ToLocal(&enable_foreign_keys_v)) {
221+
return;
222+
}
223+
if (!enable_foreign_keys_v->IsUndefined()) {
224+
if (!enable_foreign_keys_v->IsBoolean()) {
225+
node::THROW_ERR_INVALID_ARG_TYPE(
226+
env->isolate(),
227+
"The \"options.enableForeignKeyConstraints\" argument must be a "
228+
"boolean.");
229+
return;
230+
}
231+
enable_foreign_keys = enable_foreign_keys_v.As<Boolean>()->Value();
232+
}
203233
}
204234

205-
new DatabaseSync(env, args.This(), args[0].As<String>(), open);
235+
new DatabaseSync(
236+
env, args.This(), args[0].As<String>(), open, enable_foreign_keys);
206237
}
207238

208239
void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {

src/node_sqlite.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ class DatabaseSync : public BaseObject {
2323
DatabaseSync(Environment* env,
2424
v8::Local<v8::Object> object,
2525
v8::Local<v8::String> location,
26-
bool open);
26+
bool open,
27+
bool enable_foreign_keys_on_open);
2728
void MemoryInfo(MemoryTracker* tracker) const override;
2829
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
2930
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -52,6 +53,7 @@ class DatabaseSync : public BaseObject {
5253
std::unordered_set<StatementSync*> statements_;
5354

5455
friend class Session;
56+
bool enable_foreign_keys_on_open_;
5557
};
5658

5759
class StatementSync : public BaseObject {

test/parallel/test-sqlite-database-sync.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,42 @@ suite('DatabaseSync() constructor', () => {
5050
message: /The "options\.open" argument must be a boolean/,
5151
});
5252
});
53+
54+
test('throws if options.enableForeignKeyConstraints is provided but is not a boolean', (t) => {
55+
t.assert.throws(() => {
56+
new DatabaseSync('foo', { enableForeignKeyConstraints: 5 });
57+
}, {
58+
code: 'ERR_INVALID_ARG_TYPE',
59+
message: /The "options\.enableForeignKeyConstraints" argument must be a boolean/,
60+
});
61+
});
62+
63+
test('enables foreign key constraints by default', (t) => {
64+
const dbPath = nextDb();
65+
const db = new DatabaseSync(dbPath);
66+
db.exec(`
67+
CREATE TABLE foo (id INTEGER PRIMARY KEY);
68+
CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id));
69+
`);
70+
t.after(() => { db.close(); });
71+
t.assert.throws(() => {
72+
db.exec('INSERT INTO bar (foo_id) VALUES (1)');
73+
}, {
74+
code: 'ERR_SQLITE_ERROR',
75+
message: 'FOREIGN KEY constraint failed',
76+
});
77+
});
78+
79+
test('allows disabling foreign key constraints', (t) => {
80+
const dbPath = nextDb();
81+
const db = new DatabaseSync(dbPath, { enableForeignKeyConstraints: false });
82+
db.exec(`
83+
CREATE TABLE foo (id INTEGER PRIMARY KEY);
84+
CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id));
85+
`);
86+
t.after(() => { db.close(); });
87+
db.exec('INSERT INTO bar (foo_id) VALUES (1)');
88+
});
5389
});
5490

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

0 commit comments

Comments
 (0)