#14204 closed New feature (fixed)
Take advantage of SQLite support for FK constraints
Reported by: | Ramiro Morales | Owned by: | Claude Paroz |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | sqlite, foreign key |
Cc: | me@…, Simon Charette | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
SQLite3 3.6.19 from Oct 14 2009 adds support for enforcing these constraints. See http://www.sqlite.org/foreignkeys.html
Creating this ticket to track this feature, with an initial implementation patch to get feedback. Will also open a django-dev thread.
Attachments (1)
Change History (27)
by , 14 years ago
Attachment: | 14204-sqlite-fk-constraints.diff added |
---|
comment:2 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Accepted on principle; however, we need to be clear on the backwards compatibility consequences of this change. In particular, I'm concerned about automatically enabling "PRAGMA foreign_keys" just because the feature is available.
comment:3 by , 14 years ago
Summary: | Take advantage of (newish) SQLlite support for FK constraints → Take advantage of (newish) SQLite support for FK constraints |
---|
This is the django-dev mailing list thread: http://groups.google.com/group/django-developers/browse_thread/thread/fcda59f4ae21a6fd
comment:4 by , 14 years ago
Keywords: | sqlite foreign key added |
---|---|
Severity: | → Normal |
Type: | → New feature |
comment:8 by , 11 years ago
Description: | modified (diff) |
---|
comment:10 by , 10 years ago
Patch needs improvement: | set |
---|---|
Summary: | Take advantage of (newish) SQLite support for FK constraints → Take advantage of SQLite support for FK constraints |
5 years later (by the time Django 1.8 will be released), I think we could probably add this unconditionally and say that SQLite versions that don't support FKs aren't supported. Of course, we will still have to consider existing databases for which support isn't enabled.
comment:11 by , 10 years ago
How about we add a database OPTION key for SQLite to allow disabling foreign keys (mainly usable for backwards compatibility). If "disable foreign keys" flag isn't set, then starting in Django 1.8 automatically enable foreign key support.
comment:12 by , 10 years ago
Since it's called an ORM, I'm theoretically in favour of focusing on relational software. Realistically, the main problem is MyISAM. It was MySQL's default until 5.5. As long as we can't drop the code that works around databases not enforcing constraints, there's less to gain by requiring FKs on SQLite.
I'm +1 on enabling FK checks by default in Django and +0 on requiring it unconditionnally.
comment:13 by , 10 years ago
+1 on getting this patch merged in. For what it's worth, the patch's implementation of _check_fk_constraint_support
looks correct to me based on my reading of the SQLite documentation. My only question about the patch is whether the PRAGMA
needs to be issued on every new connection as the patch does, or only once when the database is created.
comment:14 by , 10 years ago
If you'd like to see it merged, feel free to update it to apply cleanly and add documentation (release notes). Then send a pull request and update the ticket flags so the patch appears in the review queue.
comment:15 by , 10 years ago
The pragma is per-connection. It's a runtime property of the SQLite engine. It doesn't affect the on-disk file format.
comment:16 by , 10 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
I'm working on finishing of this patch
comment:17 by , 8 years ago
Owner: | changed from | to
---|
comment:18 by , 8 years ago
There is one major issue with supporting FK constraints on SQLite3: they can only be deactivated outside a transaction (this is a documented behavior).
Take for example the Django TestCase implementation which wraps every test class inside a transaction, that means that foreign keys checking can never be deactivated inside TestCase tests (loading forward references in fixtures is problematic for example, as does fixtures_regress.tests.TestFixtures.test_loaddata_works_when_fixture_has_forward_refs
).
WIP work in this commit: https://github.com/claudep/django/commit/e7a9252aa6f990c4687656e9ea0f1d1ca70dc043
comment:19 by , 8 years ago
Cc: | added |
---|
Is there a reason SQLite foreign key constraints are not created DEFERRABLE INITIALLY DEFERRED
like they are on PostgreSQL and Oracle?
If they were the issue mentioned by Claude regarding transaction wrapping could be solved.
comment:20 by , 8 years ago
I don't have the time to investigate right now, but if it can help someone, I rebased the commit which is now https://github.com/claudep/django/commit/15ceb2cc05262fe5f24f16cc31367fe6b5b698c1
comment:21 by , 7 years ago
I've worked on updating Claude's patch by incorporating Simon's suggestion of adding DEFERRABLE INITIALLY DEFERRED
: PR
There are still some issues to figure out.
comment:22 by , 7 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Patch needs improvement: | unset |
The PR linked in the previous comment is now ready for review.
First implementation