mirror of
https://github.com/nodejs/node.git
synced 2024-11-21 10:59:27 +00:00
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: https://github.com/nodejs/node/pull/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>
This commit is contained in:
parent
20d8b85d34
commit
a49abec6c3
@ -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.
|
||||
|
||||
@ -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
|
||||
|
@ -91,12 +91,14 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) {
|
||||
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();
|
||||
@ -125,6 +127,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;
|
||||
}
|
||||
|
||||
@ -166,6 +177,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
|
||||
}
|
||||
|
||||
bool open = true;
|
||||
bool enable_foreign_keys = true;
|
||||
|
||||
if (args.Length() > 1) {
|
||||
if (!args[1]->IsObject()) {
|
||||
@ -188,9 +200,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) {
|
||||
|
@ -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);
|
||||
void MemoryInfo(MemoryTracker* tracker) const override;
|
||||
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
|
||||
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
|
||||
@ -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 {
|
||||
|
@ -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()', () => {
|
||||
|
Loading…
Reference in New Issue
Block a user