#31475 closed Bug (invalid)
Recursion issue when deleting related objects
Reported by: | Matt Drew | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (3)
comment:1 by , 5 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:3 by , 5 years ago
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?
Duplicate of #31435.
In short your code never supported field deferring and was causing silent queries on model initialization.
e.g.
Was causing
2N+1
database queries whereN = ClassB.objects.count()
.There's more details in the linked issue but you should be overriding
from_db
to achieve this purpose or useself.__dict__.get
to account for the fact the field might be deferred.