Opened 14 years ago

Closed 7 years ago

Last modified 6 years ago

#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 Ramiro Morales)

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)

14204-sqlite-fk-constraints.diff (5.0 KB ) - added by Ramiro Morales 14 years ago.
First implementation

Download all attachments as: .zip

Change History (27)

by Ramiro Morales, 14 years ago

First implementation

comment:1 by Ramiro Morales, 14 years ago

Needs documentation: set
Needs tests: set

See also #5698 and #12210.

comment:2 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

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 Ramiro Morales, 14 years ago

Summary: Take advantage of (newish) SQLlite support for FK constraintsTake advantage of (newish) SQLite support for FK constraints

comment:4 by Graham King, 14 years ago

Keywords: sqlite foreign key added
Severity: Normal
Type: New feature

comment:5 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:7 by Simon Charette, 11 years ago

#19659 was a duplicate.

comment:8 by Ramiro Morales, 11 years ago

Description: modified (diff)

comment:9 by Ramiro Morales <cramm0@…>, 11 years ago

In 5782c94f2382303dd28a35a9b9591ad480286220:

Added generation of SQLite FK DDL in initial migrations.

SQLite accepts the relevant standard SQL (although by default it doesn't
enforce the constraint), and the 'traditional' creation backend helper
generate it, so this allows us to:

  • Maintain the status quo
  • Improve readability of the SQL code generated for that backend.

Also, we will need this for when we fix Refs #14204.

comment:10 by Tim Graham, 10 years ago

Patch needs improvement: set
Summary: Take advantage of (newish) SQLite support for FK constraintsTake 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 Anssi Kääriäinen, 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 Aymeric Augustin, 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 William Schwartz, 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 Tim Graham, 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 Aymeric Augustin, 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 Andriy Sokolovskiy, 10 years ago

Cc: me@… added
Owner: changed from nobody to Andriy Sokolovskiy
Status: newassigned

I'm working on finishing of this patch

comment:17 by Claude Paroz, 8 years ago

Owner: changed from Andriy Sokolovskiy to Claude Paroz

comment:18 by Claude Paroz, 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 Simon Charette, 8 years ago

Cc: Simon Charette 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 Claude Paroz, 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 Tim Graham, 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 Tim Graham, 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.

comment:23 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 169c3b3:

Fixed #14204 -- Enforced SQLite foreign key constraints.

Thanks Tim Graham for contributing to the patch and
Simon Charette for advice and review.

comment:24 by Carlton Gibson <carlton.gibson@…>, 6 years ago

In 1939dd4:

Fixed #29928 -- Enabled deferred constraint checks on SQLite 3.20+.

Refs #11665, #14204.

Thanks Michel Samia for the report.

comment:25 by Tim Graham <timograham@…>, 6 years ago

In ce8b65a:

Fixed #30054 -- Implemented cascaded flush on SQLite.

This is required to maintain foreign key integrity when using
TransactionTestCase.available_apps.

Refs #30033, #14204, #20483.

comment:26 by Tim Graham <timograham@…>, 6 years ago

In 7534e434:

Refs #14204 -- Removed obsolete referential integrity comment for SQLite.

Note: See TracTickets for help on using tickets.
Back to Top