[Django] #31475: Recursion issue when deleting related objects
Django
-------------------------------------+-------------------------------------
Reporter: Matt Drew | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 3.0
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I ran into this situation yesterday... Basically, if you have a cascaded
action such as a SET_NULL on a foreign key, you could end up with a
RecursionError.
In English: ClassA is a thing. ClassB has a foreign key to ClassA with
on_delete=SET_NULL. ClassC has a foreign key to ClassB with
on_delete=CASCADE. When a ClassA is deleted, a query to get all the ClassB
objects that link to ClassA happens in order to update the foreign key to
NULL. This query is optimized to only fetch the id field.
That works fine by itself, but when ClassB has an __init__() that
references at least two non-id fields, you get into a loop. When the
objects are constructed further queries will be required to fetch the
fields referenced in __init__(), but each of those only tries to fetch a
single field and instantiate a ClassB with that field, whereupon it finds
that it also needs a second ClassB field, so it queries for that field
(and ONLY that field), which tries to instantiate a ClassB object with
just the second field, which in __init__() requires the first field so it
starts another query for the first field, ...
{{{
class ClassA(models.Model):
some_value = models.CharField(max_length=30)
class ClassB(models.Model):
foo = models.CharField(max_length=30)
bar = models.CharField(max_length=30)
my_a = models.ForeignKey(ClassA, null=True, on_delete=SET_NULL)
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self._initial_foo = self.foo # Reason for needing this is not
shown
self._initial_bar = self.bar
class ClassC(models.Model):
baz = models.CharField(max_length=30)
my_b = models.ForeignKey(ClassB, null=True, on_delete=CASCADE)
a = ClassA(some_value='hi bob')
a.save()
b = ClassB(my_a=a, foo='foo', bar='bar')
b.save()
c = ClassC(my_b=b, baz='baz')
c.save()
ClassA.objects.all().delete()
}}}
Result: RecursionError
If I add a bogus post-delete to ClassB, then django will fetch all the
ClassB fields in the query to update the ClassB.my_a references to NULL
when a ClassA is deleted, and the recursion doesn't happen.
{{{
@receiver(models.signals.post_delete, sender=ClassB)
def do_nothing(sender, *args, **kwargs):
pass
}}}
I'm not sure how best to address this. Maybe some Meta field that lets
you specify the minimum fields needed to instantiate an object? Maybe just
don't try to only fetch the id column? I'll leave it to you.
--
Ticket URL: <https://code.djangoproject.com/ticket/31475>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Matt Drew | Owner: nobody
Component: Database layer | Version: 3.0
(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
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
Duplicate of #31435.
In short your code never supported field deferring and was causing silent
queries on model initialization.
e.g.
{{{#!python
list(ClassB.objects.only('pk'))
}}}
Was causing `2N+1` database queries where `N = ClassB.objects.count()`.
There's more details in the linked issue but you should be overriding
`from_db` to achieve this purpose or use `self.__dict__.get` to account
for the fact the field might be deferred.
--
Ticket URL: <https://code.djangoproject.com/ticket/31475#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Drew | Owner: nobody
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Matt Drew):
Thanks, Simon!
--
Ticket URL: <https://code.djangoproject.com/ticket/31475#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Matt Drew | Owner: nobody
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Given we've had two reports due to `__init__` overrides so far we might
want to consider disabling the optimization introduced in
f110de5c04818b8f915dcf65da37a50c1424c6e6 when `model_cls.__init__ is not
Model.__init__` or `post_init` receivers are attached.
I'm kind of torn here though because accessing field unconditionally
during model initialization will break down the road somewhere else but
learning about this misuse through a recursion error traceback is
definitely confusing. Thoughts?
--
Ticket URL: <https://code.djangoproject.com/ticket/31475#comment:3>