[Django] #30054: SQLite doesn't implement cascading flush and breaks available_apps feature.
Django
-------------------------------------+-------------------------------------
Reporter: Simon | Owner: Simon Charette
Charette |
Type: Bug | Status: assigned
Component: Testing | Version: master
framework |
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
#30033 started performing constraint checks on schema alteration commits
on SQLite and uncovered a data integrity issue of our SQLite backend
flushing of data.
When `TransactionTestCase.available_apps` is specfied only the specified
apps subset of tested model tables are flushed for performance reasons.
Thing is when this happens all rows referring to ''flushed'' table rows
must also be flushed as specified by the
`DatabaseOperations.sql_flush(allow_cascade)` flag that SQLite completely
ignores.
This issue has been around since foreign keys were enabled on SQLite
because `execute_sql_flush` happens to disable constraint checks and allow
foreign key integrity to be silently broken.
In order to address this issue `sql_flush` must consider `allow_cascade`
and `execute_sql_flush` should stop disabling constraints.
More context from
https://github.com/django/django/pull/10779#issuecomment-449483709
--
Ticket URL: <https://code.djangoproject.com/ticket/30054>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30054#comment:1>
Django
-------------------------------------+-------------------------------------
| Charette
Component: Testing framework | Version: master
Keywords: | 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:"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/30054#comment:2>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
This slows the Django test suite by about 30-35% on my local machine and
on Jenkins.
--
Ticket URL: <https://code.djangoproject.com/ticket/30054#comment:3>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Is most of the time spent doing the recursive CTE or applying the
generated `DELETE`s?
I guess we could flush all tables when `tables and allow_cascade` or cache
relationship graphs per connection to avoid performing the CTE on each
`TransactionTestCase` teardown.
--
Ticket URL: <https://code.djangoproject.com/ticket/30054#comment:4>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Looks like about 1/4 of the time increase is for the deletes and 3/4 of
the time is for the CTEs.
--
Ticket URL: <https://code.djangoproject.com/ticket/30054#comment:5>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: fixed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Significant 30-35% speed up implemented in
https://github.com/django/django/pull/10792.
--
Ticket URL: <https://code.djangoproject.com/ticket/30054#comment:6>
Django
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: fixed
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:"2b2ae4eeb75ea53613a0992f3927e65f89affcce" 2b2ae4ee]:
{{{
#!CommitTicketReference repository=""
revision="2b2ae4eeb75ea53613a0992f3927e65f89affcce"
Refs #30054, #20483 -- Cached SQLite references graph retrieval on
sql_flush().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30054#comment:7>