Fixed django#18343 -- Cleaned up deferred model implementation

Generic cleanup and dead code removal in deferred model field loading and model.__reduce__(). Also fixed an issue where if an inherited model with a parent field chain parent_ptr_id -> id would be deferred loaded, then accessing the id field caused caused a database query, even if the id field's value is already loaded in the parent_ptr_id field.

Author: akaariai

django/db/models/base.py

@@ -404,20 +404,14 @@ def __reduce__(self):
  # and as a result, the super call will cause an infinite recursion.
  # See #10547 and #12121.
  defers = []
- pk_val = None
  if self._deferred:
  from django.db.models.query_utils import deferred_class_factory
  factory = deferred_class_factory
  for field in self._meta.fields:
  if isinstance(self.__class__.__dict__.get(field.attname),
  DeferredAttribute):
  defers.append(field.attname)
- if pk_val is None:
- # The pk_val and model values are the same for all
- # DeferredAttribute classes, so we only need to do this
- # once.
- obj = self.__class__.__dict__[field.attname]
- model = obj.model_ref()
+ model = self._meta.proxy_for_model
  else:
  factory = simple_class_factory
  return (model_unpickle, (model, defers, factory), data)

django/db/models/query_utils.py

@@ -6,8 +6,6 @@
 circular import difficulties.
 """
 
-import weakref
-
 from django.db.backends import util
 from django.utils import tree
 
@@ -70,36 +68,39 @@ class DeferredAttribute(object):
  """
  def __init__(self, field_name, model):
  self.field_name = field_name
- self.model_ref = weakref.ref(model)
- self.loaded = False
 
  def __get__(self, instance, owner):
  """
  Retrieves and caches the value from the datastore on the first lookup.
  Returns the cached value.
  """
  from django.db.models.fields import FieldDoesNotExist
+ non_deferred_model = instance._meta.proxy_for_model
+ opts = non_deferred_model._meta
 
  assert instance is not None
- cls = self.model_ref()
  data = instance.__dict__
  if data.get(self.field_name, self) is self:
  # self.field_name is the attname of the field, but only() takes the
  # actual name, so we need to translate it here.
  try:
- cls._meta.get_field_by_name(self.field_name)
- name = self.field_name
+ f = opts.get_field_by_name(self.field_name)[0]
  except FieldDoesNotExist:
- name = [f.name for f in cls._meta.fields
- if f.attname == self.field_name][0]
- # We use only() instead of values() here because we want the
- # various data coersion methods (to_python(), etc.) to be called
- # here.
- val = getattr(
- cls._base_manager.filter(pk=instance.pk).only(name).using(
- instance._state.db).get(),
- self.field_name
- )
+ f = [f for f in opts.fields
+ if f.attname == self.field_name][0]
+ name = f.name
+ # Lets see if the field is part of the parent chain. If so we
+ # might be able to reuse the already loaded value. Refs #18343.
+ val = self._check_parent_chain(instance, name)
+ if val is None:
+ # We use only() instead of values() here because we want the
+ # various data coersion methods (to_python(), etc.) to be
+ # called here.
+ val = getattr(
+ non_deferred_model._base_manager.only(name).using(
+ instance._state.db).get(pk=instance.pk),
+ self.field_name
+ )
  data[self.field_name] = val
  return data[self.field_name]
 
@@ -110,6 +111,20 @@ def __set__(self, instance, value):
  """
  instance.__dict__[self.field_name] = value
 
+ def _check_parent_chain(self, instance, name):
+ """
+ Check if the field value can be fetched from a parent field already 
+ loaded in the instance. This can be done if the to-be fetched
+ field is a primary key field.
+ """
+ opts = instance._meta
+ f = opts.get_field_by_name(name)[0]
+ link_field = opts.get_ancestor_link(f.model)
+ if f.primary_key and f != link_field:
+ return getattr(instance, link_field.attname)
+ return None
+
+
 def select_related_descend(field, restricted, requested, reverse=False):
  """
  Returns True if this field should be used to descend deeper for

tests/modeltests/defer/tests.py

@@ -158,3 +158,18 @@ def test_defer_proxy(self):
  self.assert_delayed(child, 1)
  self.assertEqual(child.name, 'p1')
  self.assertEqual(child.value, 'xx')
+
+ def test_defer_inheritance_pk_chaining(self):
+ """
+ When an inherited model is fetched from the DB, its PK is also fetched.
+ When getting the PK of the parent model it is useful to use the already
+ fetched parent model PK if it happens to be available. Tests that this
+ is done.
+ """
+ s1 = Secondary.objects.create(first="x1", second="y1")
+ bc = BigChild.objects.create(name="b1", value="foo", related=s1,
+ other="bar")
+ bc_deferred = BigChild.objects.only('name').get(pk=bc.pk)
+ with self.assertNumQueries(0):
+ bc_deferred.id
+ self.assertEqual(bc_deferred.pk, bc_deferred.id)

tests/modeltests/field_subclassing/tests.py

@@ -17,6 +17,10 @@ def test_defer(self):
  self.assertTrue(isinstance(d.data, list))
  self.assertEqual(d.data, [1, 2, 3])
 
+ d = DataModel.objects.defer("data").get(pk=d.pk)
+ self.assertTrue(isinstance(d.data, list))
+ self.assertEqual(d.data, [1, 2, 3])
+ # Refetch for save
  d = DataModel.objects.defer("data").get(pk=d.pk)
  d.save()