#36779: DeleteModel can lead to missing FK constraints
--------------------------------+--------------------------------------
Reporter: Jamie Cockburn | Owner: Vishy Algo
Type: Bug | Status: assigned
Component: Migrations | Version: 6.0
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
--------------------------------+--------------------------------------
Comment (by Simon Charette):
Well I'm not sure how I missed it in my initial analysis (maybe something
got cached in my `django-docker-box` setup) but it appears that removing
the `CASCADE` from `DROM TABLE` actually breaks a ton of tests so there
might be more to it than I initially thought.
Most of the failures seem to point at an innaproprite use of
`delete_model` in test teardown though and not at an actual problem in
migrations (which explicitly drop foreign keys first).
The following patch might be a more realistic approach to the problem
{{{#!diff
diff --git a/django/db/backends/base/schema.py
b/django/db/backends/base/schema.py
index 1f27d6a0d4..6982d094bf 100644
--- a/django/db/backends/base/schema.py
+++ b/django/db/backends/base/schema.py
@@ -86,7 +86,8 @@ class BaseDatabaseSchemaEditor:
sql_create_table = "CREATE TABLE %(table)s (%(definition)s)"
sql_rename_table = "ALTER TABLE %(old_table)s RENAME TO
%(new_table)s"
sql_retablespace_table = "ALTER TABLE %(table)s SET TABLESPACE
%(new_tablespace)s"
- sql_delete_table = "DROP TABLE %(table)s CASCADE"
+ sql_delete_table = "DROP TABLE %(table)s"
+ sql_delete_table_cascade = "DROP TABLE %(table)s CASCADE"
sql_create_column = "ALTER TABLE %(table)s ADD COLUMN %(column)s
%(definition)s"
sql_alter_column = "ALTER TABLE %(table)s %(changes)s"
@@ -538,7 +539,7 @@ class BaseDatabaseSchemaEditor:
if field.remote_field.through._meta.auto_created:
self.create_model(field.remote_field.through)
- def delete_model(self, model):
+ def delete_model(self, model, *, cascade=False):
"""Delete a model from the database."""
# Handle auto-created intermediary models
for field in model._meta.local_many_to_many:
@@ -546,8 +547,9 @@ class BaseDatabaseSchemaEditor:
self.delete_model(field.remote_field.through)
# Delete the table
+ delete_sql = self.sql_delete_table_cascade if cascade else
self.sql_delete_table
self.execute(
- self.sql_delete_table
+ delete_sql
% {
"table": self.quote_name(model._meta.db_table),
}
diff --git a/django/db/backends/oracle/schema.py
b/django/db/backends/oracle/schema.py
index 13fa7220ce..281aa1db7b 100644
--- a/django/db/backends/oracle/schema.py
+++ b/django/db/backends/oracle/schema.py
@@ -23,7 +23,7 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
"CONSTRAINT %(name)s REFERENCES
%(to_table)s(%(to_column)s)%(on_delete_db)"
"s%(deferrable)s"
)
- sql_delete_table = "DROP TABLE %(table)s CASCADE CONSTRAINTS"
+ sql_delete_table_cascade = "DROP TABLE %(table)s CASCADE CONSTRAINTS"
sql_create_index = "CREATE INDEX %(name)s ON %(table)s
(%(columns)s)%(extra)s"
def quote_value(self, value):
@@ -47,9 +47,9 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
self._drop_identity(model._meta.db_table, field.column)
super().remove_field(model, field)
- def delete_model(self, model):
+ def delete_model(self, model, *, cascade=False):
# Run superclass action
- super().delete_model(model)
+ super().delete_model(model, cascade=cascade)
# Clean up manually created sequence.
self.execute(
"""
diff --git a/django/db/backends/sqlite3/schema.py
b/django/db/backends/sqlite3/schema.py
index ee6163c253..6313b9be43 100644
--- a/django/db/backends/sqlite3/schema.py
+++ b/django/db/backends/sqlite3/schema.py
@@ -10,7 +10,8 @@ from django.db.models import CompositePrimaryKey,
UniqueConstraint
class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
- sql_delete_table = "DROP TABLE %(table)s"
+ # SQLite doesn't support DROP TABLE CASCADE.
+ sql_delete_table_cascade = "DROP TABLE %(table)s"
sql_create_fk = None
sql_create_inline_fk = (
"REFERENCES %(to_table)s (%(to_column)s)%(on_delete_db)s
DEFERRABLE INITIALLY "
@@ -278,9 +279,9 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
if restore_pk_field:
restore_pk_field.primary_key = True
- def delete_model(self, model, handle_autom2m=True):
+ def delete_model(self, model, handle_autom2m=True, *, cascade=False):
if handle_autom2m:
- super().delete_model(model)
+ super().delete_model(model, cascade=cascade)
else:
# Delete the table (and only that)
self.execute(
diff --git a/tests/schema/tests.py b/tests/schema/tests.py
index ab8b07e9d3..2aaafc93e3 100644
--- a/tests/schema/tests.py
+++ b/tests/schema/tests.py
@@ -159,7 +159,7 @@ class SchemaTests(TransactionTestCase):
if self.isolated_local_models:
with connection.schema_editor() as editor:
for model in self.isolated_local_models:
- editor.delete_model(model)
+ editor.delete_model(model, cascade=True)
def delete_tables(self):
"Deletes all model tables for our models for a clean test
environment"
@@ -174,7 +174,7 @@ class SchemaTests(TransactionTestCase):
if connection.features.ignores_table_name_case:
tbl = tbl.lower()
if tbl in table_names:
- editor.delete_model(model)
+ editor.delete_model(model, cascade=True)
table_names.remove(tbl)
connection.enable_constraint_checking()
@@ -1542,7 +1542,7 @@ class SchemaTests(TransactionTestCase):
def drop_collation():
with connection.cursor() as cursor:
- cursor.execute(f"DROP COLLATION IF EXISTS
{ci_collation}")
+ cursor.execute(f"DROP COLLATION IF EXISTS {ci_collation}
CASCADE")
with connection.cursor() as cursor:
cursor.execute(
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36779#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/0107019b0e6d6698-17626007-0035-4df4-a688-025ff9fad5cd-000000%40eu-central-1.amazonses.com.
