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
This commit is contained in:
Malcolm Tredinnick 2009-04-04 03:21:31 +00:00
parent 19d1450929
commit dded5f52cc
5 changed files with 63 additions and 45 deletions

View File

@ -184,12 +184,12 @@ class RelatedGeoModelTest(unittest.TestCase):
self.assertEqual(m.point, t[1]) self.assertEqual(m.point, t[1])
# Test disabled until #10572 is resolved. # Test disabled until #10572 is resolved.
#def test08_defer_only(self): def test08_defer_only(self):
# "Testing defer() and only() on Geographic models." "Testing defer() and only() on Geographic models."
# qs = Location.objects.all() qs = Location.objects.all()
# def_qs = Location.objects.defer('point') def_qs = Location.objects.defer('point')
# for loc, def_loc in zip(qs, def_qs): for loc, def_loc in zip(qs, def_qs):
# self.assertEqual(loc.point, def_loc.point) self.assertEqual(loc.point, def_loc.point)
# TODO: Related tests for KML, GML, and distance lookups. # TODO: Related tests for KML, GML, and distance lookups.

View File

@ -362,9 +362,8 @@ class Model(object):
# DeferredAttribute classes, so we only need to do this # DeferredAttribute classes, so we only need to do this
# once. # once.
obj = self.__class__.__dict__[field.attname] obj = self.__class__.__dict__[field.attname]
pk_val = obj.pk_value
model = obj.model_ref() model = obj.model_ref()
return (model_unpickle, (model, pk_val, defers), data) return (model_unpickle, (model, defers), data)
def _get_pk_val(self, meta=None): def _get_pk_val(self, meta=None):
if not meta: if not meta:
@ -635,12 +634,12 @@ def get_absolute_url(opts, func, self, *args, **kwargs):
class Empty(object): class Empty(object):
pass pass
def model_unpickle(model, pk_val, attrs): def model_unpickle(model, attrs):
""" """
Used to unpickle Model subclasses with deferred fields. Used to unpickle Model subclasses with deferred fields.
""" """
from django.db.models.query_utils import deferred_class_factory from django.db.models.query_utils import deferred_class_factory
cls = deferred_class_factory(model, pk_val, attrs) cls = deferred_class_factory(model, attrs)
return cls.__new__(cls) return cls.__new__(cls)
model_unpickle.__safe_for_unpickle__ = True model_unpickle.__safe_for_unpickle__ = True

View File

@ -190,6 +190,20 @@ class QuerySet(object):
index_start = len(extra_select) index_start = len(extra_select)
aggregate_start = index_start + len(self.model._meta.fields) aggregate_start = index_start + len(self.model._meta.fields)
load_fields = only_load.get(self.model)
skip = None
if load_fields and not fill_cache:
# Some fields have been deferred, so we have to initialise
# via keyword arguments.
skip = set()
init_list = []
for field in fields:
if field.name not in load_fields:
skip.add(field.attname)
else:
init_list.append(field.attname)
model_cls = deferred_class_factory(self.model, skip)
for row in self.query.results_iter(): for row in self.query.results_iter():
if fill_cache: if fill_cache:
obj, _ = get_cached_row(self.model, row, obj, _ = get_cached_row(self.model, row,
@ -197,25 +211,10 @@ class QuerySet(object):
requested=requested, offset=len(aggregate_select), requested=requested, offset=len(aggregate_select),
only_load=only_load) only_load=only_load)
else: else:
load_fields = only_load.get(self.model) if skip:
if load_fields:
# Some fields have been deferred, so we have to initialise
# via keyword arguments.
row_data = row[index_start:aggregate_start] row_data = row[index_start:aggregate_start]
pk_val = row_data[pk_idx] pk_val = row_data[pk_idx]
skip = set() obj = model_cls(**dict(zip(init_list, row_data)))
init_list = []
for field in fields:
if field.name not in load_fields:
skip.add(field.attname)
else:
init_list.append(field.attname)
if skip:
model_cls = deferred_class_factory(self.model, pk_val,
skip)
obj = model_cls(**dict(zip(init_list, row_data)))
else:
obj = self.model(*row[index_start:aggregate_start])
else: else:
# Omit aggregates in object creation. # Omit aggregates in object creation.
obj = self.model(*row[index_start:aggregate_start]) 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,
else: else:
init_list.append(field.attname) init_list.append(field.attname)
if skip: if skip:
klass = deferred_class_factory(klass, pk_val, skip) klass = deferred_class_factory(klass, skip)
obj = klass(**dict(zip(init_list, fields))) obj = klass(**dict(zip(init_list, fields)))
else: else:
obj = klass(*fields) obj = klass(*fields)

View File

@ -158,9 +158,8 @@ class DeferredAttribute(object):
A wrapper for a deferred-loading field. When the value is read from this A wrapper for a deferred-loading field. When the value is read from this
object the first time, the query is executed. object the first time, the query is executed.
""" """
def __init__(self, field_name, pk_value, model): def __init__(self, field_name, model):
self.field_name = field_name self.field_name = field_name
self.pk_value = pk_value
self.model_ref = weakref.ref(model) self.model_ref = weakref.ref(model)
self.loaded = False self.loaded = False
@ -170,21 +169,18 @@ class DeferredAttribute(object):
Returns the cached value. Returns the cached value.
""" """
assert instance is not None assert instance is not None
if not self.loaded: cls = self.model_ref()
obj = self.model_ref() data = instance.__dict__
if obj is None: if data.get(self.field_name, self) is self:
return data[self.field_name] = cls._base_manager.filter(pk=instance.pk).values_list(self.field_name, flat=True).get()
self.value = list(obj._base_manager.filter(pk=self.pk_value).values_list(self.field_name, flat=True))[0] return data[self.field_name]
self.loaded = True
return self.value
def __set__(self, name, value): def __set__(self, instance, value):
""" """
Deferred loading attributes can be set normally (which means there will Deferred loading attributes can be set normally (which means there will
never be a database lookup involved. never be a database lookup involved.
""" """
self.value = value instance.__dict__[self.field_name] = value
self.loaded = True
def select_related_descend(field, restricted, requested): def select_related_descend(field, restricted, requested):
""" """
@ -206,7 +202,7 @@ def select_related_descend(field, restricted, requested):
# This function is needed because data descriptors must be defined on a class # This function is needed because data descriptors must be defined on a class
# object, not an instance, to have any effect. # object, not an instance, to have any effect.
def deferred_class_factory(model, pk_value, attrs): def deferred_class_factory(model, attrs):
""" """
Returns a class object that is a copy of "model" with the specified "attrs" Returns a class object that is a copy of "model" with the specified "attrs"
being replaced with DeferredAttribute objects. The "pk_value" ties the being replaced with DeferredAttribute objects. The "pk_value" ties the
@ -223,7 +219,7 @@ def deferred_class_factory(model, pk_value, attrs):
# are identical. # are identical.
name = "%s_Deferred_%s" % (model.__name__, '_'.join(sorted(list(attrs)))) name = "%s_Deferred_%s" % (model.__name__, '_'.join(sorted(list(attrs))))
overrides = dict([(attr, DeferredAttribute(attr, pk_value, model)) overrides = dict([(attr, DeferredAttribute(attr, model))
for attr in attrs]) for attr in attrs])
overrides["Meta"] = Meta overrides["Meta"] = Meta
overrides["__module__"] = model.__module__ overrides["__module__"] = model.__module__
@ -233,4 +229,3 @@ def deferred_class_factory(model, pk_value, attrs):
# The above function is also used to unpickle model instances with deferred # The above function is also used to unpickle model instances with deferred
# fields. # fields.
deferred_class_factory.__safe_for_unpickling__ = True deferred_class_factory.__safe_for_unpickling__ = True

View File

@ -6,7 +6,7 @@ from django.conf import settings
from django.db import connection, models from django.db import connection, models
class Item(models.Model): class Item(models.Model):
name = models.CharField(max_length=10) name = models.CharField(max_length=15)
text = models.TextField(default="xyzzy") text = models.TextField(default="xyzzy")
value = models.IntegerField() value = models.IntegerField()
other_value = models.IntegerField(default=0) other_value = models.IntegerField(default=0)
@ -14,6 +14,9 @@ class Item(models.Model):
def __unicode__(self): def __unicode__(self):
return self.name return self.name
class RelatedItem(models.Model):
item = models.ForeignKey(Item)
__test__ = {"regression_tests": """ __test__ = {"regression_tests": """
Deferred fields should really be deferred and not accidentally use the field's Deferred fields should really be deferred and not accidentally use the field's
default value just because they aren't passed to __init__. default value just because they aren't passed to __init__.
@ -39,9 +42,31 @@ True
u"xyzzy" u"xyzzy"
>>> len(connection.queries) == num + 2 # Effect of text lookup. >>> len(connection.queries) == num + 2 # Effect of text lookup.
True True
>>> obj.text
u"xyzzy"
>>> len(connection.queries) == num + 2
True
>>> settings.DEBUG = False >>> settings.DEBUG = False
Regression test for #10695. Make sure different instances don't inadvertently
share data in the deferred descriptor objects.
>>> i = Item.objects.create(name="no I'm first", value=37)
>>> items = Item.objects.only('value').order_by('-value')
>>> items[0].name
u'first'
>>> items[1].name
u"no I'm first"
>>> _ = RelatedItem.objects.create(item=i)
>>> r = RelatedItem.objects.defer('item').get()
>>> r.item_id == i.id
True
>>> r.item == i
True
""" """
} }