Opened 9 years ago
Closed 5 years ago
#25361 closed Bug (fixed)
Unpickling of QuerySet fails in presence of abstract intermediate model
Reported by: | Torsten Bronger | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | bronger@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (6)
comment:1 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 8 years ago
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.
Edit: I've submitted a PR for this: https://github.com/django/django/pull/7280
Edit 2: The Above PR was closed - but if you need to get this working, you can pretty easily override the pickle method *without* having to Monkey Patch! - https://gist.github.com/LegoStormtroopr/f014801e63474971e7ffc23980f0b1d1
comment:3 by , 5 years ago
I believe this got fixed somewhat accidentally.
Using the same model definitions as in the report, I wrote the following testcase:
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.
comment:4 by , 5 years ago
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.
comment:6 by , 5 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Fixed in 67cf5efa31acb2916034afb15610b700695dfcb0.
I guess we should special case fields of abstract models in 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.