[Django] #36428: Collector.delete() calls sort() but does not order deletions correctly when nullable FK is present with non-null value

[Django]

#36428: Collector.delete() calls sort() but does not order deletions correctly when
nullable FK is present with non-null value
-------------------------------------+-------------------------------------
Reporter: Andréas Kühne | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
When using DeleteView or Model.delete() in Django 5.1.8, I encountered a
IntegrityError from PostgreSQL. The issue stems from the deletion order of
models: Django attempts to delete a parent (ActivityLocation) before its
dependent child (BookableItem), despite on_delete=models.CASCADE being
set.

This occurs even though the Collector calls .sort(), which is supposed to
reorder models in dependency-safe order. The BookableItem.location FK is
nullable (null=True), but the actual instance has a non-null FK set.

Models to reproduce:

{{{
from django.db import models

class ActivityLocation(models.Model):
name = models.CharField(max_length=100)

class BookableItem(models.Model):
name = models.CharField(max_length=100)
location = models.ForeignKey(
ActivityLocation,
on_delete=models.CASCADE,
related_name='bookable_items',
null=True,
blank=True,
)

}}}

Steps to reproduce:

{{{
# Create parent and child
location = ActivityLocation.objects.create(name="Test Location")
item = BookableItem.objects.create(name="Test Item", location=location)

# Try to delete the location
location.delete()
}}}

What I think should happen:
BookableItem should be deleted before ActivityLocation, as Django is
responsible for enforcing deletion order (Postgres won’t do it
automatically with deferred constraints).

What actually happens:
Django runs Collector.delete() which internally calls sort(), but the
model deletion order remains:

{{{
[ActivityLocation, BookableItem]
}}}

If I however change to this:


{{{
from django.db import models

class BookableItem(models.Model):
name = models.CharField(max_length=100)
location = models.ForeignKey(
ActivityLocation,
on_delete=models.CASCADE,
related_name='bookable_items',
)
}}}

Then the code works as expected.
--
Ticket URL: <https://code.djangoproject.com/ticket/36428>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36428: Collector.delete() calls sort() but does not order deletions correctly when
nullable FK is present with non-null value
-------------------------------------+-------------------------------------

Reporter: Andréas Kühne | Owner: (none)
Type: Bug | Status: closed
Component: Database layer | Version: 5.1
(models, ORM) |
Severity: Normal | Resolution: invalid

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* resolution: => invalid
* status: new => closed

Comment:

Hello Andréas, I unfortunately cannot reproduce against `main`, `5.2.x`,
or `5.1.6` with the following test against SQLite and Postgres

{{{#!diff
diff --git a/tests/delete/models.py b/tests/delete/models.py
index 4b627712bb..024466fef6 100644
--- a/tests/delete/models.py
+++ b/tests/delete/models.py
@@ -241,3 +241,18 @@ class GenericDeleteBottomParent(models.Model):
generic_delete_bottom = models.ForeignKey(
GenericDeleteBottom, on_delete=models.CASCADE
)
+
+
+class ActivityLocation(models.Model):
+ name = models.CharField(max_length=100)
+
+
+class BookableItem(models.Model):
+ name = models.CharField(max_length=100)
+ location = models.ForeignKey(
+ ActivityLocation,
+ on_delete=models.CASCADE,
+ related_name="bookable_items",
+ null=True,
+ blank=True,
+ )
diff --git a/tests/delete/tests.py b/tests/delete/tests.py
index 01228631f4..c0c03411c7 100644
--- a/tests/delete/tests.py
+++ b/tests/delete/tests.py
@@ -4,7 +4,12 @@
from django.db.models import ProtectedError, Q, RestrictedError
from django.db.models.deletion import Collector
from django.db.models.sql.constants import GET_ITERATOR_CHUNK_SIZE
-from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature
+from django.test import (
+ TestCase,
+ TransactionTestCase,
+ skipIfDBFeature,
+ skipUnlessDBFeature,
+)

from .models import (
B1,
@@ -800,3 +805,17 @@ def test_fast_delete_full_match(self):
with self.assertNumQueries(1):
User.objects.filter(~Q(pk__in=[]) |
Q(avatar__desc="foo")).delete()
self.assertFalse(User.objects.exists())
+
+
+class Ticket36428Tests(TransactionTestCase):
+ available_apps = ["delete"]
+
+ def test_order(self):
+ from .models import ActivityLocation, BookableItem
+
+ location = ActivityLocation.objects.create(name="Test Location")
+ item = BookableItem.objects.create(name="Test Item",
location=location)
+
+ # Try to delete the location
+ location.delete()
}}}

In all cases the issued SQL is correct and of the form

{{{#!sql
BEGIN;
---
DELETE
FROM "delete_bookableitem"
WHERE "delete_bookableitem"."location_id" IN (1);
---
DELETE
FROM "delete_activitylocation"
WHERE "delete_activitylocation"."id" IN (1);
---
COMMIT;
}}}

It would be quite surprising if cascade deletion dependency resolving was
broken since 5.1.x given the way it's extensively tested and used in the
wild so I highly suspect something else is at play here.

Perhaps you have model signal receivers connected (`post_save`,
`post_delete`) that might be interfering?
--
Ticket URL: <https://code.djangoproject.com/ticket/36428#comment:1>