Skip to content

Commit 67f5f46

Browse files
tniessenaduh95
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: #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 f703652 commit 67f5f46

File tree

4 files changed

+78
-3
lines changed

4 files changed

+78
-3
lines changed

doc/api/sqlite.md

+6
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

@@ -317,6 +322,7 @@ exception.
317322

318323
[SQL injection]: https://en.wikipedia.org/wiki/SQL_injection
319324
[`--experimental-sqlite`]: cli.md#--experimental-sqlite
325+
[`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys
320326
[`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html
321327
[`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html
322328
[`sqlite3_exec()`]: https://www.sqlite.org/c3ref/exec.html

src/node_sqlite.cc

+33-2
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,14 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) {
9191
DatabaseSync::DatabaseSync(Environment* env,
9292
Local<Object> object,
9393
Local<String> location,
94-
bool open)
94+
bool open,
95+
bool enable_foreign_keys_on_open)
9596
: BaseObject(env, object) {
9697
MakeWeak();
9798
node::Utf8Value utf8_location(env->isolate(), location);
9899
location_ = utf8_location.ToString();
99100
connection_ = nullptr;
101+
enable_foreign_keys_on_open_ = enable_foreign_keys_on_open;
100102

101103
if (open) {
102104
Open();
@@ -125,6 +127,15 @@ bool DatabaseSync::Open() {
125127
int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
126128
int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, nullptr);
127129
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
130+
131+
int foreign_keys_enabled;
132+
r = sqlite3_db_config(connection_,
133+
SQLITE_DBCONFIG_ENABLE_FKEY,
134+
static_cast<int>(enable_foreign_keys_on_open_),
135+
&foreign_keys_enabled);
136+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
137+
CHECK_EQ(foreign_keys_enabled, enable_foreign_keys_on_open_);
138+
128139
return true;
129140
}
130141

@@ -166,6 +177,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
166177
}
167178

168179
bool open = true;
180+
bool enable_foreign_keys = true;
169181

170182
if (args.Length() > 1) {
171183
if (!args[1]->IsObject()) {
@@ -188,9 +200,28 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
188200
}
189201
open = open_v.As<Boolean>()->Value();
190202
}
203+
204+
Local<String> enable_foreign_keys_string =
205+
FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints");
206+
Local<Value> enable_foreign_keys_v;
207+
if (!options->Get(env->context(), enable_foreign_keys_string)
208+
.ToLocal(&enable_foreign_keys_v)) {
209+
return;
210+
}
211+
if (!enable_foreign_keys_v->IsUndefined()) {
212+
if (!enable_foreign_keys_v->IsBoolean()) {
213+
node::THROW_ERR_INVALID_ARG_TYPE(
214+
env->isolate(),
215+
"The \"options.enableForeignKeyConstraints\" argument must be a "
216+
"boolean.");
217+
return;
218+
}
219+
enable_foreign_keys = enable_foreign_keys_v.As<Boolean>()->Value();
220+
}
191221
}
192222

193-
new DatabaseSync(env, args.This(), args[0].As<String>(), open);
223+
new DatabaseSync(
224+
env, args.This(), args[0].As<String>(), open, enable_foreign_keys);
194225
}
195226

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

src/node_sqlite.h

+3-1
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

+36
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)