Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#30054 closed Bug (fixed)

SQLite doesn't implement cascading flush and breaks available_apps feature.

Reported by: Simon Charette Owned by: Simon Charette
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

#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

Change History (7)

comment:1 Changed 9 months ago by Simon Charette

Has patch: set
Version 0, edited 9 months ago by Simon Charette (next)

comment:2 Changed 9 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

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:3 Changed 9 months ago by Tim Graham

This slows the Django test suite by about 30-35% on my local machine and on Jenkins.

comment:4 Changed 9 months ago by Simon Charette

Is most of the time spent doing the recursive CTE or applying the generated DELETEs?

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.

comment:5 Changed 9 months ago 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.

comment:6 Changed 9 months ago by Simon Charette

Significant 30-35% speed up implemented in https://github.com/django/django/pull/10792.

comment:7 Changed 9 months ago by Tim Graham <timograham@…>

In 2b2ae4ee:

Refs #30054, #20483 -- Cached SQLite references graph retrieval on sql_flush().

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