[Django] #25361: Unpickling of QuerySet fails in presence of abstract intermediate model
Django
----------------------------------------------+--------------------
Reporter: bronger | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Consider the following model hierarchy:
{{{
from django.db import models
class Pizza(models.Model):
timestamp = models.DateTimeField("last modified", auto_now=True)
class SpecialPizza(Pizza):
class Meta:
abstract = True
ordering = ["timestamp"]
class MyPizza(SpecialPizza):
pass
class Topping(models.Model):
pizza = models.ForeignKey(MyPizza, related_name="toppings")
class Meta:
ordering = ["pizza"]
}}}
QuerySet objects containing MyPizza objects with Toppings cannot be
pickled and unpickled:
{{{
my_pizza = models.MyPizza.objects.create()
my_pizza.toppings.add(models.Topping())
pickled = pickle.dumps(my_pizza.toppings.all())
response = pickle.loads(pickled)
}}}
The traceback is:
{{{
Traceback:
File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py"
in get_response
132. response = wrapped_callback(request,
*callback_args, **callback_kwargs)
File "/usr/local/lib/python2.7/dist-packages/django/utils/decorators.py"
in inner
145. return func(*args, **kwargs)
File "/tmp/myproject/myproject/myapp/views.py" in test
9. response = pickle.loads(pickled)
File "/usr/lib/python2.7/pickle.py" in loads
1383. return Unpickler(file).load()
File "/usr/lib/python2.7/pickle.py" in load
858. dispatch[key](self)
File "/usr/lib/python2.7/pickle.py" in load_reduce
1134. value = func(*args)
File "/usr/local/lib/python2.7/dist-
packages/django/db/models/fields/__init__.py" in _load_field
66. return apps.get_model(app_label,
model_name)._meta.get_field(field_name)
File "/usr/local/lib/python2.7/dist-packages/django/apps/registry.py" in
get_model
202. return
self.get_app_config(app_label).get_model(model_name.lower())
File "/usr/local/lib/python2.7/dist-packages/django/apps/config.py" in
get_model
162. "App '%s' doesn't have a '%s' model." %
(self.label, model_name))
Exception Type: LookupError at /
Exception Value: App 'myapp' doesn't have a 'specialpizza' model.
}}}
If one removes one of the "ordering" fields, the unpickler uses "MyPizza"
instead of "SpecialPizza", which is correct and should always be the case.
--
Ticket URL: <https://code.djangoproject.com/ticket/25361>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: bronger | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
I guess we should special case fields of abstract models in
[https://github.com/django/django/blob/a7901c2e090d720ef0427552387be7f14cbe2ac6/django/db/models/fields/__init__.py#L504-L522
Field.__reduce__].
The code added in #19635 could be either adjusted to raise a
`RuntimeError` or return another unpickling function that relies on the
`__path__` of the abstract model to retrieve the associated class and call
`_meta.get_field()` on it.
--
Ticket URL: <https://code.djangoproject.com/ticket/25361#comment:1>
Django
Reporter: Torsten Bronger | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Samuel Spencer):
I just got bit by this and I can't say I've come across the best solution,
but I was able to resolve it with this code in
{{{/django/db/models/fields/__init__.py}}}:
{{{
--- __init__.py 2016-09-22 06:07:36.167725186 +0000
+++ __init__.py.old 2016-09-22 06:08:19.175742789 +0000
@@ -66,11 +66,6 @@
return apps.get_model(app_label,
model_name)._meta.get_field(field_name)
-def _load_field_for_abstract(app_label, model_name, field_name):
- from django.utils.module_loading import import_string
- return import_string('%s.models.%s'%(app_label,
model_name))._meta.get_field(field_name)
-
-
# A guide to Field parameters:
#
# * name: The name of the field specified in the model.
@@ -508,11 +503,7 @@
# Deferred model will not be found from the app registry.
This
# could be fixed by reconstructing the deferred model on
unpickle.
raise RuntimeError("Fields of deferred models can't be
reduced")
- if self.model._meta.abstract:
- func = _load_field_for_abstract
- else:
- func = _load_field
- return func, (self.model._meta.app_label,
self.model._meta.object_name,
+ return _load_field, (self.model._meta.app_label,
self.model._meta.object_name,
self.name)
def get_pk_value_on_save(self, instance):
}}}
{{{__init__.py.old}}} is the original from Django 1.8.14. I'm not sure
where to begin with submitting and testing this, but I'd like to help if
someone could give me some pointers.
--
Ticket URL: <https://code.djangoproject.com/ticket/25361#comment:2>
Django
Reporter: Torsten Bronger | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Baptiste Mispelon):
I believe this got fixed somewhat accidentally.
Using the same model definitions as in the report, I wrote the following
testcase:
{{{#!python
class TicketTestCase(TestCase):
def test_ticket(self):
my_pizza = MyPizza.objects.create()
my_pizza.toppings.create()
pickled = pickle.dumps(my_pizza.toppings.all())
with self.assertRaises(LookupError):
response = pickle.loads(pickled)
}}}
This allowed me to bisect and find the commit that apparently fixed the
issue: 67cf5efa31acb2916034afb15610b700695dfcb0.
I'm not very familiar with pickling in Python and that commit doesn't seem
to have anything to do with pickling (though it does relate to abstract
models) so I'm a bit hesitant to mark this ticket as fixed.
--
Ticket URL: <https://code.djangoproject.com/ticket/25361#comment:3>
Django
Reporter: Torsten Bronger | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
Thanks for the sleuthing Baptiste!
I think 67cf5efa31acb2916034afb15610b700695dfcb0 happened to address the
crash because it replaced parent links previously bound to abstract models
to the actual concrete model subclass. Since only concrete models are
registered the `apps.get_model(app_label, model_name)` now correctly
works.
--
Ticket URL: <https://code.djangoproject.com/ticket/25361#comment:4>
Django
Reporter: Torsten Bronger | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"200cd8803d51e94a0724f9fcd3782e71752ef424" 200cd880]:
{{{
#!CommitTicketReference repository=""
revision="200cd8803d51e94a0724f9fcd3782e71752ef424"
Refs #25361 -- Added test for pickling queryset of abstract-inherited
models with Meta.ordering.
Fixed in 67cf5efa31acb2916034afb15610b700695dfcb0.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25361#comment:5>
Django
Reporter: Torsten Bronger | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* status: new => closed
* resolution: => fixed
Comment:
Fixed in 67cf5efa31acb2916034afb15610b700695dfcb0.
--
Ticket URL: <https://code.djangoproject.com/ticket/25361#comment:6>