Skip to content

Commit c4692b7

Browse files
tniessentpoisseau
authored andcommitted
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 <yagiz@nizipli.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent ca8dedb commit c4692b7

4 files changed

Lines changed: 78 additions & 3 deletions

File tree

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

@@ -336,6 +341,7 @@ exception.
336341

337342
[SQL injection]: https://en.wikipedia.org/wiki/SQL_injection
338343
[`--experimental-sqlite`]: cli.md#--experimental-sqlite
344+
[`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys
339345
[`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html
340346
[`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html
341347
[`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();
@@ -127,6 +129,15 @@ bool DatabaseSync::Open() {
127129
int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
128130
int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, nullptr);
129131
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
132+
133+
int foreign_keys_enabled;
134+
r = sqlite3_db_config(connection_,
135+
SQLITE_DBCONFIG_ENABLE_FKEY,
136+
static_cast<int>(enable_foreign_keys_on_open_),
137+
&foreign_keys_enabled);
138+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
139+
CHECK_EQ(foreign_keys_enabled, enable_foreign_keys_on_open_);
140+
130141
return true;
131142
}
132143

@@ -168,6 +179,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
168179
}
169180

170181
bool open = true;
182+
bool enable_foreign_keys = true;
171183

172184
if (args.Length() > 1) {
173185
if (!args[1]->IsObject()) {
@@ -190,9 +202,28 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
190202
}
191203
open = open_v.As<Boolean>()->Value();
192204
}
205+
206+
Local<String> enable_foreign_keys_string =
207+
FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints");
208+
Local<Value> enable_foreign_keys_v;
209+
if (!options->Get(env->context(), enable_foreign_keys_string)
210+
.ToLocal(&enable_foreign_keys_v)) {
211+
return;
212+
}
213+
if (!enable_foreign_keys_v->IsUndefined()) {
214+
if (!enable_foreign_keys_v->IsBoolean()) {
215+
node::THROW_ERR_INVALID_ARG_TYPE(
216+
env->isolate(),
217+
"The \"options.enableForeignKeyConstraints\" argument must be a "
218+
"boolean.");
219+
return;
220+
}
221+
enable_foreign_keys = enable_foreign_keys_v.As<Boolean>()->Value();
222+
}
193223
}
194224

195-
new DatabaseSync(env, args.This(), args[0].As<String>(), open);
225+
new DatabaseSync(
226+
env, args.This(), args[0].As<String>(), open, enable_foreign_keys);
196227
}
197228

198229
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
@@ -21,7 +21,8 @@ class DatabaseSync : public BaseObject {
2121
DatabaseSync(Environment* env,
2222
v8::Local<v8::Object> object,
2323
v8::Local<v8::String> location,
24-
bool open);
24+
bool open,
25+
bool enable_foreign_keys_on_open);
2526
void MemoryInfo(MemoryTracker* tracker) const override;
2627
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
2728
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -43,6 +44,7 @@ class DatabaseSync : public BaseObject {
4344
std::string location_;
4445
sqlite3* connection_;
4546
std::unordered_set<StatementSync*> statements_;
47+
bool enable_foreign_keys_on_open_;
4648
};
4749

4850
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)