Re: [Django] #14204: Take advantage of (newish) SQLite support for FK constraints
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: sqlite, foreign key | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by charettes):
#19659 was a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: sqlite, foreign key | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Old description:
> 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 of this feature, with an initial
> implementation patch to get feedback. Will also open a django-dev thread.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: sqlite, foreign key | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Ramiro Morales <cramm0@…>):
In [changeset:"5782c94f2382303dd28a35a9b9591ad480286220"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: sqlite, foreign key | Needs documentation: 1
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: sqlite, foreign key | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 1
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: sqlite, foreign key | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 1
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Since it's called an O''R''M, 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by wkschwartz):
+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 [http://www.sqlite.org/foreignkeys.html#fk_enable
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
The pragma is per-connection. It's a runtime property of the SQLite
engine. It doesn't affect the on-disk file format.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:15>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => coldmind
* status: new => assigned
* cc: me@… (added)
Comment:
I'm working on finishing of this patch
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:16>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: coldmind => claudep
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: ramiro | Owner: claudep
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by claudep):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:18>
Django
-------------------------------------+-------------------------------------
| Paroz
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: Ramiro Morales | Owner: Claude
| Paroz
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Claude Paroz):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: Ramiro Morales | Owner: Claude
| Paroz
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I've worked on updating Claude's patch by incorporating Simon's suggestion
of adding `DEFERRABLE INITIALLY DEFERRED`:
[https://github.com/django/django/pull/8678 PR]
There are still some issues to figure out.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: Ramiro Morales | Owner: Claude
| Paroz
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sqlite, foreign key | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
The PR linked in the previous comment is now ready for review.
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: Ramiro Morales | Owner: Claude
| Paroz
Component: Database layer | Version: master
(models, ORM) |
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"169c3b3e07829d9ffa409b6eb5c1094d8ef918a8" 169c3b3]:
{{{
#!CommitTicketReference repository=""
revision="169c3b3e07829d9ffa409b6eb5c1094d8ef918a8"
Fixed #14204 -- Enforced SQLite foreign key constraints.
Thanks Tim Graham for contributing to the patch and
Simon Charette for advice and review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: Ramiro Morales | Owner: Claude
| Paroz
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson <carlton.gibson@…>):
In [changeset:"1939dd49d142b65fa22eb5f85cee0d20864d3730" 1939dd4]:
{{{
#!CommitTicketReference repository=""
revision="1939dd49d142b65fa22eb5f85cee0d20864d3730"
Fixed #29928 -- Enabled deferred constraint checks on SQLite 3.20+.
Refs #11665, #14204.
Thanks Michel Samia for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: Ramiro Morales | Owner: Claude
| Paroz
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"ce8b65ac5e8e20912c36ab5c714ddf11de3b664c" ce8b65a]:
{{{
#!CommitTicketReference repository=""
revision="ce8b65ac5e8e20912c36ab5c714ddf11de3b664c"
Fixed #30054 -- Implemented cascaded flush on SQLite.
This is required to maintain foreign key integrity when using
TransactionTestCase.available_apps.
Refs #30033, #14204, #20483.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: Ramiro Morales | Owner: Claude
| Paroz
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: sqlite, foreign key | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"7534e43497fed7714910ccec2a6e8acade8263ca" 7534e434]:
{{{
#!CommitTicketReference repository=""
revision="7534e43497fed7714910ccec2a6e8acade8263ca"
Refs #14204 -- Removed obsolete referential integrity comment for SQLite.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14204#comment:26>