#31435 closed Bug (invalid)
RecursionError when deleting a model with one to many relationship.
Reported by: | Fabian Allendorf | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.0 |
Severity: | Normal | Keywords: | delete model defer recursionerror |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This issue appeared with upgrade from Django 2 to Django 3.0. I double checked and it definitely works with version <3.0.
There is a model with a duration field and the "init" method is overridden to save "old" values of this duration field.
class TimeSeries(models.Model): frequency = models.DurationField() def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._old_frequency = self.frequency
Another model extends this "TimeSeries" model and has a foreign key reference to the model which I tried to delete.
class RefModel(TimeSeries): main_model = models.ForeignKey( MainModel, on_delete=models.CASCADE, related_name="refs" ) class MainModel(models.Model): pass
So I created an instance of "MainModel" "main_a" and added some "RefModel" instances with relationhip to "main_a".
When I call "main_a.delete()" it ends up in a stack overflow with the following stacktrace:
Traceback (most recent call last): File "<input>", line 1, in <module> File "/lib/python3.8/site-packages/django/db/models/base.py", line 937, in delete collector.collect([self], keep_parents=keep_parents) File "/lib/python3.8/site-packages/django/db/models/deletion.py", line 244, in collect if sub_objs: File "/lib/python3.8/site-packages/django/db/models/query.py", line 280, in __bool__ self._fetch_all() File "/lib/python3.8/site-packages/django/db/models/query.py", line 1261, in _fetch_all self._result_cache = list(self._iterable_class(self)) File "/lib/python3.8/site-packages/django/db/models/query.py", line 75, in __iter__ obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end]) File "/lib/python3.8/site-packages/django/db/models/base.py", line 512, in from_db new = cls(*values) File "/lib/python3.8/site-packages/django_timeseries/models.py", line 27, in __init__ self._old_frequency = self.frequency File "/webserver/business_rules/thread_safe_model_patcher.py", line 51, in _patched_get_attribute return Model.get(model_self, item) File "/lib/python3.8/site-packages/django/db/models/query_utils.py", line 139, in __get__ instance.refresh_from_db(fields=[field_name]) File "/lib/python3.8/site-packages/django/db/models/base.py", line 627, in refresh_from_db db_instance = db_instance_qs.get() File "/lib/python3.8/site-packages/django/db/models/query.py", line 411, in get num = len(clone) File "/lib/python3.8/site-packages/django/db/models/query.py", line 258, in __len__ self._fetch_all() File "/lib/python3.8/site-packages/django/db/models/query.py", line 1261, in _fetch_all <--- Stacktrace repeats itself from here on --->
"django_timeseries" is a custom package which defines the "TimeSeries" class.
"thread_safe_model_patcher.py" monkeypatches the "Model" class. It has no effect on whether the error happens. I removed the this patch and it still errors.
It seems to me that deleting "main_a" deletes all "RefModel" instances related to it (like it should) but while doing this, it gets stuck initializing the super class "TimeSeries" because the "init" accesses the "frequency" field.
Change History (8)
comment:1 by , 5 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
Summary: | RecursionError when deleting a model with one to many relationship → RecursionError when deleting a model with one to many relationship. |
follow-up: 4 comment:2 by , 5 years ago
Judging by the trace of the exception it seems like you already forked django-timeseries
or are using a different package with the same name since the Pypi one doesn't have a models.py
file.
In all cases your code was broken because you are accessing a deferred field in your TimeSeries.__init__
model which is not allowed.
You can test it out but on any version of Django doing TimeSeries.objects.defer('frequency')
would cause N + 1
queries to be performed where N = TimeSeries.objects.count()
.
You want to do self._old_frequency = self.__dict__.get('frequency')
in there.
For the record the commit that introduced the recursion error for your bogus Model.__init__
is f110de5c04818b8f915dcf65da37a50c1424c6e6 which started deferring fields of model when possible to avoid fetching large amount of data when unnecessary on deletion (#30191).
It is also documented that the right way to implement custom model loading should be done using Model.from_db()
. I guess you could assign django.db.models.DEFERRED
to _old_frequency
in there and special case it's access when necessary instead of assigning None
to it.
comment:3 by , 5 years ago
Keywords: | defer recursionerror added; recursion stack overflow reference foreign key one to many removed |
---|
comment:4 by , 5 years ago
Replying to Simon Charette:
Judging by the trace of the exception it seems like you already forked
django-timeseries
or are using a different package with the same name since the Pypi one doesn't have amodels.py
file.
In all cases your code was broken because you are accessing a deferred field in your
TimeSeries.__init__
model which is not allowed.
You can test it out but on any version of Django doing
TimeSeries.objects.defer('frequency')
would causeN + 1
queries to be performed whereN = TimeSeries.objects.count()
.
You want to do
self._old_frequency = self.__dict__.get('frequency')
in there.
The package django-timseries
caused a lot of confusion it seems. I should have added that its a private package we have developed at our company and not this one https://pypi.org/project/django-timeseries/.
Thank you, I think that is exactly the point I was missing.
comment:5 by , 5 years ago
Resolution: | needsinfo → invalid |
---|
django-timeseries
looks abandoned and it doesn't support Django 2.0+. Moreover I was not able to reproduce this issue with a sample projectCan you provide a sample project (without
django-timeseries
) that reproduces this issue?