moigagoo / norm

A Nim ORM for SQLite and Postgres
https://norm.nim.town
MIT License
381 stars 34 forks source link

[Bug?] SQLITE-Foreignkeys pragma is off by default whenever a new connection is opened and acts on a per connection basis #184

Closed PhilippMDoerner closed 1 year ago

PhilippMDoerner commented 1 year ago

What I'm about to describe is an issue with **sqlite***, not with norm itself. Norm could however possibly address what I deem unexpected behaviour. I just noticed this one because I found a bug in my database that should've been quite literally impossible.

The Problem

Here a reproduction with a minimal example: ```sql DROP TABLE IF EXISTS "campaign"; DROP TABLE IF EXISTS "creature"; CREATE TABLE "campaign" ( "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "name" varchar(200) NOT NULL UNIQUE ); CREATE TABLE "creature" ( "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "name" varchar(200) NOT NULL UNIQUE, "campaign_id" REFERENCES "campaign" ("id") DEFERRABLE INITIALLY DEFERRED ); INSERT INTO "campaign" VALUES (1, "MyCampaign"); INSERT INTO "creature" VALUES (1, "cre1", 1), (2, "cre2", 1); DELETE FROM "campaign" WHERE id = 1; ``` If you run this, you will NOT get an error, even though you're deleting the entry "MyCampaign" which is referenced by 2 other entries! That is because foreign key checks are not turned on by default, that is done via the PRAGMA statement. `PRAGMA foreign_keys=on;` So by default norm (because of the way sqlite behaves) does not do foreign key checks before deleting, inserting or updating an entry. Now you might **think** that this just needs to be done once on the db file and then it works in perpetuity, [Think again](https://sqlite.org/forum/info/c5dc50f61b88c587). That PRAGMA needs to be turned on on a **per connection** basis. Here a minimal example using db_sqlite to prove this (though you can also try this out with the sqlite3 binary by connecting to a dbfile, enabling the pragma, disconnecting and then connecting again, it displays the exact same behaviour): ```nim import std/db_sqlite let db1 = open("fk.sqlite3", "", "", "") let db2 = open("fk.sqlite3", "", "", "") proc printPragma(con: DbConn) = echo con.getRow(sql"PRAGMA foreign_keys;") db1.printPragma() # @["0"] <-- Pragma is off db1.exec(sql"PRAGMA foreign_keys=on;") db1.printPragma() # @["1"] <-- Pragma is on db2.printPragma() # @["0"] <-- Pragma is off again ?! ```

Proposed solution

I find foreign key checks being always off unless specifically turned on to be unexpected and problematic behaviour. I would like there to be the option to automatically execute `"PRAGMA foreign_keys=on;"` on every connection so that I don't have to think about it when I want to ensure data correctness in my sqlite database. This could be done in various ways, so here some suggestions: 1) Provide an option, at least for the sqlite pool, to execute user defined SQL statements immediately after a connection is created 2) Provide an enum of options, at least for the sqlite pool, that represent pre-defined SQL statements to execute immediately after a connection is created 3) Provide a boolean specifically for this statement that allows this to be turned on (or off, imo that thing should be turned on by default) Alternatively of course we could also leave this out altogether. The reason I propose this change is because it is entirely unexpected behaviour. If I turn on the FK Pragma, I expect it to be there when I use a different connection. Naturally that is a misconception, but I would like it if we could "hide" this issue from sqlite in the first place, this is not something users should be forced to deal with.
moigagoo commented 1 year ago

How about we just enable the pragma for all connections with no option to disable it? Norm isn't fully operational without it anyway since you can't have the functionality related to foreign keys at all.

PhilippMDoerner commented 1 year ago

Sounds reasonable to me!