Opened 7 weeks ago

Closed 7 weeks ago

Last modified 7 weeks ago

#36428 closed Bug (invalid)

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

Reported by: Andréas Kühne Owned by:
Component: Database layer (models, ORM) Version: 5.1
Severity: Normal Keywords:
Cc: Andréas Kühne Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (1)

comment:1 by Simon Charette, 7 weeks ago

Resolution: invalid
Status: newclosed

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

  • tests/delete/models.py

    diff --git a/tests/delete/models.py b/tests/delete/models.py
    index 4b627712bb..024466fef6 100644
    a b class GenericDeleteBottomParent(models.Model):  
    241241    generic_delete_bottom = models.ForeignKey(
    242242        GenericDeleteBottom, on_delete=models.CASCADE
    243243    )
     244
     245
     246class ActivityLocation(models.Model):
     247    name = models.CharField(max_length=100)
     248
     249
     250class BookableItem(models.Model):
     251    name = models.CharField(max_length=100)
     252    location = models.ForeignKey(
     253        ActivityLocation,
     254        on_delete=models.CASCADE,
     255        related_name="bookable_items",
     256        null=True,
     257        blank=True,
     258    )
  • tests/delete/tests.py

    diff --git a/tests/delete/tests.py b/tests/delete/tests.py
    index 01228631f4..c0c03411c7 100644
    a b  
    44from django.db.models import ProtectedError, Q, RestrictedError
    55from django.db.models.deletion import Collector
    66from django.db.models.sql.constants import GET_ITERATOR_CHUNK_SIZE
    7 from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature
     7from django.test import (
     8    TestCase,
     9    TransactionTestCase,
     10    skipIfDBFeature,
     11    skipUnlessDBFeature,
     12)
    813
    914from .models import (
    1015    B1,
    def test_fast_delete_full_match(self):  
    800805        with self.assertNumQueries(1):
    801806            User.objects.filter(~Q(pk__in=[]) | Q(avatar__desc="foo")).delete()
    802807        self.assertFalse(User.objects.exists())
     808
     809
     810class Ticket36428Tests(TransactionTestCase):
     811    available_apps = ["delete"]
     812
     813    def test_order(self):
     814        from .models import ActivityLocation, BookableItem
     815
     816        location = ActivityLocation.objects.create(name="Test Location")
     817        item = BookableItem.objects.create(name="Test Item", location=location)
     818
     819        # Try to delete the location
     820        location.delete()

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

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 (pre_delete, post_delete) that might be interfering?

Last edited 7 weeks ago by Simon Charette (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top