Fixed #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.

4 files changed, 52 insertions, 24 deletions

diff --git a/django/db/models/base.py b/django/db/models/base.py
index 08bfc73a87..13238fc9dc 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -404,7 +404,6 @@ class Model(object):
         # 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
@@ -412,12 +411,7 @@ class Model(object):
                 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)
diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py
index a56ab5ca80..5676fdce9a 100644
--- a/django/db/models/query_utils.py
+++ b/django/db/models/query_utils.py
@@ -6,8 +6,6 @@ large and/or so that they can be used by other modules without getting into
 circular import difficulties.
 """
 
-import weakref
-
 from django.db.backends import util
 from django.utils import tree
 
@@ -70,8 +68,6 @@ 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):
         """
@@ -79,27 +75,32 @@ class DeferredAttribute(object):
         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 @@ class DeferredAttribute(object):
         """
         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
diff --git a/tests/modeltests/defer/tests.py b/tests/modeltests/defer/tests.py
index 09138293af..eb09162b01 100644
--- a/tests/modeltests/defer/tests.py
+++ b/tests/modeltests/defer/tests.py
@@ -158,3 +158,18 @@ class DeferTests(TestCase):
         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)
diff --git a/tests/modeltests/field_subclassing/tests.py b/tests/modeltests/field_subclassing/tests.py
index c6a2335f37..48755123f2 100644
--- a/tests/modeltests/field_subclassing/tests.py
+++ b/tests/modeltests/field_subclassing/tests.py
@@ -18,6 +18,10 @@ class CustomField(TestCase):
         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()
 
         d = DataModel.objects.get(pk=d.pk)