Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #10695 -- Fixed implementation of deferred attribute retrieval.

The original implementation had a few silly bugs in it that meant that data was
not being used only on the instance of the class that it was appropriate for
(one of the traps when using class-level things). No more!

Thanks to Justin Bronn and Alex Gaynor for the patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@10382 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit dded5f52cc1dbe0e58b64a3217e12e72da7bed23 1 parent 19d1450
Malcolm Tredinnick authored April 04, 2009
12  django/contrib/gis/tests/relatedapp/tests.py
@@ -184,12 +184,12 @@ def test07_values(self):
184 184
             self.assertEqual(m.point, t[1])
185 185
 
186 186
     # Test disabled until #10572 is resolved.
187  
-    #def test08_defer_only(self):
188  
-    #    "Testing defer() and only() on Geographic models."
189  
-    #    qs = Location.objects.all()
190  
-    #    def_qs = Location.objects.defer('point')
191  
-    #    for loc, def_loc in zip(qs, def_qs):
192  
-    #        self.assertEqual(loc.point, def_loc.point)
  187
+    def test08_defer_only(self):
  188
+        "Testing defer() and only() on Geographic models."
  189
+        qs = Location.objects.all()
  190
+        def_qs = Location.objects.defer('point')
  191
+        for loc, def_loc in zip(qs, def_qs):
  192
+            self.assertEqual(loc.point, def_loc.point)
193 193
 
194 194
     # TODO: Related tests for KML, GML, and distance lookups.
195 195
 
7  django/db/models/base.py
@@ -362,9 +362,8 @@ def __reduce__(self):
362 362
                     # DeferredAttribute classes, so we only need to do this
363 363
                     # once.
364 364
                     obj = self.__class__.__dict__[field.attname]
365  
-                    pk_val = obj.pk_value
366 365
                     model = obj.model_ref()
367  
-        return (model_unpickle, (model, pk_val, defers), data)
  366
+        return (model_unpickle, (model, defers), data)
368 367
 
369 368
     def _get_pk_val(self, meta=None):
370 369
         if not meta:
@@ -635,12 +634,12 @@ def get_absolute_url(opts, func, self, *args, **kwargs):
635 634
 class Empty(object):
636 635
     pass
637 636
 
638  
-def model_unpickle(model, pk_val, attrs):
  637
+def model_unpickle(model, attrs):
639 638
     """
640 639
     Used to unpickle Model subclasses with deferred fields.
641 640
     """
642 641
     from django.db.models.query_utils import deferred_class_factory
643  
-    cls = deferred_class_factory(model, pk_val, attrs)
  642
+    cls = deferred_class_factory(model, attrs)
644 643
     return cls.__new__(cls)
645 644
 model_unpickle.__safe_for_unpickle__ = True
646 645
 
35  django/db/models/query.py
@@ -190,6 +190,20 @@ def iterator(self):
190 190
         index_start = len(extra_select)
191 191
         aggregate_start = index_start + len(self.model._meta.fields)
192 192
 
  193
+        load_fields = only_load.get(self.model)
  194
+        skip = None
  195
+        if load_fields and not fill_cache:
  196
+            # Some fields have been deferred, so we have to initialise
  197
+            # via keyword arguments.
  198
+            skip = set()
  199
+            init_list = []
  200
+            for field in fields:
  201
+                if field.name not in load_fields:
  202
+                    skip.add(field.attname)
  203
+                else:
  204
+                    init_list.append(field.attname)
  205
+            model_cls = deferred_class_factory(self.model, skip)
  206
+
193 207
         for row in self.query.results_iter():
194 208
             if fill_cache:
195 209
                 obj, _ = get_cached_row(self.model, row,
@@ -197,25 +211,10 @@ def iterator(self):
197 211
                             requested=requested, offset=len(aggregate_select),
198 212
                             only_load=only_load)
199 213
             else:
200  
-                load_fields = only_load.get(self.model)
201  
-                if load_fields:
202  
-                    # Some fields have been deferred, so we have to initialise
203  
-                    # via keyword arguments.
  214
+                if skip:
204 215
                     row_data = row[index_start:aggregate_start]
205 216
                     pk_val = row_data[pk_idx]
206  
-                    skip = set()
207  
-                    init_list = []
208  
-                    for field in fields:
209  
-                        if field.name not in load_fields:
210  
-                            skip.add(field.attname)
211  
-                        else:
212  
-                            init_list.append(field.attname)
213  
-                    if skip:
214  
-                        model_cls = deferred_class_factory(self.model, pk_val,
215  
-                                skip)
216  
-                        obj = model_cls(**dict(zip(init_list, row_data)))
217  
-                    else:
218  
-                        obj = self.model(*row[index_start:aggregate_start])
  217
+                    obj = model_cls(**dict(zip(init_list, row_data)))
219 218
                 else:
220 219
                     # Omit aggregates in object creation.
221 220
                     obj = self.model(*row[index_start:aggregate_start])
@@ -927,7 +926,7 @@ def get_cached_row(klass, row, index_start, max_depth=0, cur_depth=0,
927 926
                 else:
928 927
                     init_list.append(field.attname)
929 928
             if skip:
930  
-                klass = deferred_class_factory(klass, pk_val, skip)
  929
+                klass = deferred_class_factory(klass, skip)
931 930
                 obj = klass(**dict(zip(init_list, fields)))
932 931
             else:
933 932
                 obj = klass(*fields)
27  django/db/models/query_utils.py
@@ -158,9 +158,8 @@ class DeferredAttribute(object):
158 158
     A wrapper for a deferred-loading field. When the value is read from this
159 159
     object the first time, the query is executed.
160 160
     """
161  
-    def __init__(self, field_name, pk_value, model):
  161
+    def __init__(self, field_name, model):
162 162
         self.field_name = field_name
163  
-        self.pk_value = pk_value
164 163
         self.model_ref = weakref.ref(model)
165 164
         self.loaded = False
166 165
 
@@ -170,21 +169,18 @@ def __get__(self, instance, owner):
170 169
         Returns the cached value.
171 170
         """
172 171
         assert instance is not None
173  
-        if not self.loaded:
174  
-            obj = self.model_ref()
175  
-            if obj is None:
176  
-                return
177  
-            self.value = list(obj._base_manager.filter(pk=self.pk_value).values_list(self.field_name, flat=True))[0]
178  
-            self.loaded = True
179  
-        return self.value
180  
-
181  
-    def __set__(self, name, value):
  172
+        cls = self.model_ref()
  173
+        data = instance.__dict__
  174
+        if data.get(self.field_name, self) is self:
  175
+            data[self.field_name] = cls._base_manager.filter(pk=instance.pk).values_list(self.field_name, flat=True).get()
  176
+        return data[self.field_name]
  177
+
  178
+    def __set__(self, instance, value):
182 179
         """
183 180
         Deferred loading attributes can be set normally (which means there will
184 181
         never be a database lookup involved.
185 182
         """
186  
-        self.value = value
187  
-        self.loaded = True
  183
+        instance.__dict__[self.field_name] = value
188 184
 
189 185
 def select_related_descend(field, restricted, requested):
190 186
     """
@@ -206,7 +202,7 @@ def select_related_descend(field, restricted, requested):
206 202
 # This function is needed because data descriptors must be defined on a class
207 203
 # object, not an instance, to have any effect.
208 204
 
209  
-def deferred_class_factory(model, pk_value, attrs):
  205
+def deferred_class_factory(model, attrs):
210 206
     """
211 207
     Returns a class object that is a copy of "model" with the specified "attrs"
212 208
     being replaced with DeferredAttribute objects. The "pk_value" ties the
@@ -223,7 +219,7 @@ class Meta:
223 219
     # are identical.
224 220
     name = "%s_Deferred_%s" % (model.__name__, '_'.join(sorted(list(attrs))))
225 221
 
226  
-    overrides = dict([(attr, DeferredAttribute(attr, pk_value, model))
  222
+    overrides = dict([(attr, DeferredAttribute(attr, model))
227 223
             for attr in attrs])
228 224
     overrides["Meta"] = Meta
229 225
     overrides["__module__"] = model.__module__
@@ -233,4 +229,3 @@ class Meta:
233 229
 # The above function is also used to unpickle model instances with deferred
234 230
 # fields.
235 231
 deferred_class_factory.__safe_for_unpickling__ = True
236  
-
29  tests/regressiontests/defer_regress/models.py
@@ -6,7 +6,7 @@
6 6
 from django.db import connection, models
7 7
 
8 8
 class Item(models.Model):
9  
-    name = models.CharField(max_length=10)
  9
+    name = models.CharField(max_length=15)
10 10
     text = models.TextField(default="xyzzy")
11 11
     value = models.IntegerField()
12 12
     other_value = models.IntegerField(default=0)
@@ -14,6 +14,9 @@ class Item(models.Model):
14 14
     def __unicode__(self):
15 15
         return self.name
16 16
 
  17
+class RelatedItem(models.Model):
  18
+    item = models.ForeignKey(Item)
  19
+
17 20
 __test__ = {"regression_tests": """
18 21
 Deferred fields should really be deferred and not accidentally use the field's
19 22
 default value just because they aren't passed to __init__.
@@ -39,9 +42,31 @@ def __unicode__(self):
39 42
 u"xyzzy"
40 43
 >>> len(connection.queries) == num + 2      # Effect of text lookup.
41 44
 True
  45
+>>> obj.text
  46
+u"xyzzy"
  47
+>>> len(connection.queries) == num + 2
  48
+True
42 49
 
43 50
 >>> settings.DEBUG = False
44 51
 
  52
+Regression test for #10695. Make sure different instances don't inadvertently
  53
+share data in the deferred descriptor objects.
  54
+
  55
+>>> i = Item.objects.create(name="no I'm first", value=37)
  56
+>>> items = Item.objects.only('value').order_by('-value')
  57
+>>> items[0].name
  58
+u'first'
  59
+>>> items[1].name
  60
+u"no I'm first"
  61
+
  62
+>>> _ = RelatedItem.objects.create(item=i)
  63
+>>> r = RelatedItem.objects.defer('item').get()
  64
+>>> r.item_id == i.id
  65
+True
  66
+>>> r.item == i
  67
+True
  68
+
  69
+
  70
+
45 71
 """
46 72
 }
47  
-

0 notes on commit dded5f5

Please sign in to comment.
Something went wrong with that request. Please try again.