[Django] #31435: RecursionError when deleting a model with one to many relationship
Django
-------------------------------------+-------------------------------------
Reporter: Fabian | Owner: nobody
Allendorf |
Type: Bug | Status: new
Component: Database | Version: 3.0
layer (models, ORM) | Keywords:
Severity: Normal | delete,model,recursion,stack,overflow,reference,foreign,key,one,to,many
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31435>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
delete,model,recursion,stack,overflow,reference,foreign,key,one,to,many| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
`django-timeseries` looks abandoned and it doesn't support
[https://github.com/anthonyalmarza/django-
timeseries/blob/e7ce5ef15daad18d11e4c0e49603df7d0fff2dc5/tox.ini#L8-L11
Django 2.0+]. Moreover I was not able to reproduce this issue with a
sample project
{{{
def test_regression(self):
obj = MainModel.objects.create()
r1 = RefModel.objects.create(frequency=timedelta(1),
main_model=obj)
r2 = RefModel.objects.create(frequency=timedelta(2),
main_model=obj)
obj.delete()
}}}
Can you provide a sample project (without `django-timeseries`) that
reproduces this issue?
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
delete,model,recursion,stack,overflow,reference,foreign,key,one,to,many| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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 ones 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: needsinfo
recursionerror | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords:
delete,model,recursion,stack,overflow,reference,foreign,key,one,to,many
=> delete model defer recursionerror
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: delete model defer | Triage Stage:
recursionerror | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Fabian Allendorf):
Replying to [comment:2 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 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.
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Keywords: delete model defer | Triage Stage:
recursionerror | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: needsinfo => invalid
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: delete model defer | Triage Stage:
recursionerror | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):
In [changeset:"e47298aec4fa04416e7082331fbd44bd9f2662aa" e47298a]:
{{{
#!CommitTicketReference repository=""
revision="e47298aec4fa04416e7082331fbd44bd9f2662aa"
Refs #31435 -- Doc'd potential infinite recursion when accessing model
fields in __init__.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: delete model defer | Triage Stage:
recursionerror | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):
In [changeset:"0e34ac8981035dfefd622e7fd180e5e813d3846b" 0e34ac8]:
{{{
#!CommitTicketReference repository=""
revision="0e34ac8981035dfefd622e7fd180e5e813d3846b"
[5.0.x] Refs #31435 -- Doc'd potential infinite recursion when accessing
model fields in __init__.
Backport of e47298aec4fa04416e7082331fbd44bd9f2662aa from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Fabian Allendorf | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 3.0
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: delete model defer | Triage Stage:
recursionerror | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Natalia <124304+nessita@…>):
In [changeset:"66978802196da4ec1e4b6bf9b1094136e9c16abf" 6697880]:
{{{
#!CommitTicketReference repository=""
revision="66978802196da4ec1e4b6bf9b1094136e9c16abf"
[4.2.x] Refs #31435 -- Doc'd potential infinite recursion when accessing
model fields in __init__.
Backport of e47298aec4fa04416e7082331fbd44bd9f2662aa from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31435#comment:8>