Ticket #17485: defer_select_related-throws_exception.patch

File defer_select_related-throws_exception.patch, 12.7 KB (added by Michal Petrucha, 13 years ago)

Patch containing fixes for both issues along with tests and docs

  • django/db/models/query.py 

    diff --git a/django/db/models/query.py b/django/db/models/query.py
index 41c24c7..25f0883 100644
    a b def get_klass_info(klass, max_depth=0, cur_depth=0, requested=None,  
    12841284        # Build the list of fields that *haven't* been requested
    12851285        for field, model in klass._meta.get_fields_with_model():
    12861286            if field.name not in load_fields:
    1287                 skip.add(field.name)
     1287                skip.add(field.attname)
    12881288            elif local_only and model is not None:
    12891289                continue
    12901290            else:
    … … def get_klass_info(klass, max_depth=0, cur_depth=0, requested=None,  
    13151315
    13161316    related_fields = []
    13171317    for f in klass._meta.fields:
    1318         if select_related_descend(f, restricted, requested):
     1318        if select_related_descend(f, restricted, requested, load_fields):
    13191319            if restricted:
    13201320                next = requested[f.name]
    13211321            else:
    … … def get_klass_info(klass, max_depth=0, cur_depth=0, requested=None,  
    13271327    reverse_related_fields = []
    13281328    if restricted:
    13291329        for o in klass._meta.get_all_related_objects():
    1330             if o.field.unique and select_related_descend(o.field, restricted, requested, reverse=True):
     1330            if o.field.unique and select_related_descend(o.field, restricted, requested,
     1331                                                         only_load.get(o.model), reverse=True):
    13311332                next = requested[o.field.related_query_name()]
    13321333                klass_info = get_klass_info(o.model, max_depth=max_depth, cur_depth=cur_depth+1,
    13331334                                            requested=next, only_load=only_load, local_only=True)

  • django/db/models/query_utils.py 

    diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py
index a56ab5c..68c79e9 100644
    a b class DeferredAttribute(object):  
    110110        """
    111111        instance.__dict__[self.field_name] = value
    112112
    113 def select_related_descend(field, restricted, requested, reverse=False):
     113def select_related_descend(field, restricted, requested, load_fields, reverse=False):
    114114    """
    115115    Returns True if this field should be used to descend deeper for
    116116    select_related() purposes. Used by both the query construction code
    117117    (sql.query.fill_related_selections()) and the model instance creation code
    118     (query.get_cached_row()).
     118    (query.get_klass_info()).
    119119
    120120    Arguments:
    121121     * field - the field to be checked
    122122     * restricted - a boolean field, indicating if the field list has been
    123123       manually restricted using a requested clause)
    124124     * requested - The select_related() dictionary.
     125     * load_fields - the set of fields to be loaded on this model
    125126     * reverse - boolean, True if we are checking a reverse select related
    126127    """
    127128    if not field.rel:
    … … def select_related_descend(field, restricted, requested, reverse=False):  
    135136            return False
    136137    if not restricted and field.null:
    137138        return False
     139    if load_fields:
     140        if field.name not in load_fields:
     141            if restricted and field.name in requested:
     142                raise InvalidQuery("Field %s.%s cannot be both deferred"
     143                                   " and traversed using select_related"
     144                                   " at the same time." %
     145                                   (field.model._meta.object_name, field.name))
     146            return False
    138147    return True
    139148
    140149# This function is needed because data descriptors must be defined on a class

  • django/db/models/sql/compiler.py 

    diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index 72948f9..9474d89 100644
    a b class SQLCompiler(object):  
    595595        if avoid_set is None:
    596596            avoid_set = set()
    597597        orig_dupe_set = dupe_set
     598        only_load = self.query.get_loaded_field_names()
    598599
    599600        # Setup for the case when only particular related fields should be
    600601        # included in the related selection.
    … … class SQLCompiler(object):  
    606607                restricted = False
    607608
    608609        for f, model in opts.get_fields_with_model():
    609             if not select_related_descend(f, restricted, requested):
     610            if not select_related_descend(f, restricted, requested,
     611                                          only_load.get(model or self.query.model)):
    610612                continue
    611613            # The "avoid" set is aliases we want to avoid just for this
    612614            # particular branch of the recursion. They aren't permanently
    … … class SQLCompiler(object):  
    679681                if o.field.unique
    680682            ]
    681683            for f, model in related_fields:
    682                 if not select_related_descend(f, restricted, requested, reverse=True):
     684                if not select_related_descend(f, restricted, requested,
     685                                              only_load.get(model), reverse=True):
    683686                    continue
    684687                # The "avoid" set is aliases we want to avoid just for this
    685688                # particular branch of the recursion. They aren't permanently

  • django/db/models/sql/query.py 

    diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index ed2bc06..38ee4fe 100644
    a b class Query(object):  
    18361836
    18371837        If no fields are marked for deferral, returns an empty dictionary.
    18381838        """
    1839         collection = {}
    1840         self.deferred_to_data(collection, self.get_loaded_field_names_cb)
    1841         return collection
     1839        # We cache this because we call this function multiple times
     1840        # (compiler.fill_related_selections, query.iterator)
     1841        try:
     1842            return self._loaded_field_names_cache
     1843        except AttributeError:
     1844            collection = {}
     1845            self.deferred_to_data(collection, self.get_loaded_field_names_cb)
     1846            self._loaded_field_names_cache = collection
     1847            return collection
    18421848
    18431849    def get_loaded_field_names_cb(self, target, model, fields):
    18441850        """

  • docs/ref/models/querysets.txt 

    diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt
index 7633555..302bd0a 100644
    a b to ``defer()``::  
    10791079    # Load all fields immediately.
    10801080    my_queryset.defer(None)
    10811081
     1082.. versionchanged:: 1.4
     1083
    10821084Some fields in a model won't be deferred, even if you ask for them. You can
    10831085never defer the loading of the primary key. If you are using
    10841086:meth:`select_related()` to retrieve related models, you shouldn't defer the
    1085 loading of the field that connects from the primary model to the related one
    1086 (at the moment, that doesn't raise an error, but it will eventually).
     1087loading of the field that connects from the primary model to the related
     1088one, doing so will result in an error.
    10871089
    10881090.. note::
    10891091
    … … logically::  
    11431145    # existing set of fields).
    11441146    Entry.objects.defer("body").only("headline", "body")
    11451147
     1148.. versionchanged:: 1.4
     1149
    11461150All of the cautions in the note for the :meth:`defer` documentation apply to
    11471151``only()`` as well. Use it cautiously and only after exhausting your other
    1148 options.
     1152options. Also note that using :meth:`only` and omitting a field requested
     1153using :meth:`select_related` is an error as well.
    11491154
    11501155using
    11511156~~~~~

  • tests/modeltests/defer/tests.py 

    diff --git a/tests/modeltests/defer/tests.py b/tests/modeltests/defer/tests.py
index 542162c..7af36b3 100644
    a b  
    11from __future__ import absolute_import
    22
    3 from django.db.models.query_utils import DeferredAttribute
     3from django.db.models.query_utils import DeferredAttribute, InvalidQuery
    44from django.test import TestCase
    55
    66from .models import Secondary, Primary, Child, BigChild
    … … class DeferTests(TestCase):  
    7373        self.assert_delayed(qs.defer("name").get(pk=p1.pk), 1)
    7474        self.assert_delayed(qs.only("name").get(pk=p1.pk), 2)
    7575
    76         # DOES THIS WORK?
    77         self.assert_delayed(qs.only("name").select_related("related")[0], 1)
    78         self.assert_delayed(qs.defer("related").select_related("related")[0], 0)
     76        # When we defer a field and also select_related it, the query is
     77        # invalid and raises an exception.
     78        with self.assertRaises(InvalidQuery):
     79            qs.only("name").select_related("related")[0]
     80        with self.assertRaises(InvalidQuery):
     81            qs.defer("related").select_related("related")[0]
     82
     83        # With a depth-based select_related, all deferred ForeignKeys are
     84        # deferred instead of traversed.
     85        with self.assertNumQueries(3):
     86            obj = qs.defer("related").select_related()[0]
     87            self.assert_delayed(obj, 1)
     88            self.assertEqual(obj.related.id, s1.pk)
    7989
    8090        # Saving models with deferred fields is possible (but inefficient,
    8191        # since every field has to be retrieved first).

  • tests/regressiontests/defer_regress/models.py 

    diff --git a/tests/regressiontests/defer_regress/models.py b/tests/regressiontests/defer_regress/models.py
index 812d2da..bd4f845 100644
    a b class SimpleItem(models.Model):  
    4747
    4848class Feature(models.Model):
    4949    item = models.ForeignKey(SimpleItem)
     50
     51class ItemAndSimpleItem(models.Model):
     52    item = models.ForeignKey(Item)
     53    simple = models.ForeignKey(SimpleItem)

  • tests/regressiontests/defer_regress/tests.py 

    diff --git a/tests/regressiontests/defer_regress/tests.py b/tests/regressiontests/defer_regress/tests.py
index 4afe39b..f21f691 100644
    a b from django.db.models.loading import cache  
    99from django.test import TestCase
    1010
    1111from .models import (ResolveThis, Item, RelatedItem, Child, Leaf, Proxy,
    12     SimpleItem, Feature)
     12    SimpleItem, Feature, ItemAndSimpleItem)
    1313
    1414
    1515class DeferRegressionTest(TestCase):
    … … class DeferRegressionTest(TestCase):  
    109109                Child,
    110110                Feature,
    111111                Item,
     112                ItemAndSimpleItem,
    112113                Leaf,
    113114                Proxy,
    114115                RelatedItem,
    … … class DeferRegressionTest(TestCase):  
    125126                ),
    126127            )
    127128        )
     129        # FIXME: This is dependent on the order in which tests are run --
     130        # this test case has to be the first, otherwise a LOT more classes
     131        # appear.
    128132        self.assertEqual(
    129133            klasses, [
    130134                "Child",
    131135                "Child_Deferred_value",
    132136                "Feature",
    133137                "Item",
     138                "ItemAndSimpleItem",
    134139                "Item_Deferred_name",
    135140                "Item_Deferred_name_other_value_text",
    136141                "Item_Deferred_name_other_value_value",
    … … class DeferRegressionTest(TestCase):  
    139144                "Leaf",
    140145                "Leaf_Deferred_child_id_second_child_id_value",
    141146                "Leaf_Deferred_name_value",
    142                 "Leaf_Deferred_second_child_value",
     147                "Leaf_Deferred_second_child_id_value",
    143148                "Leaf_Deferred_value",
    144149                "Proxy",
    145150                "RelatedItem",
    … … class DeferRegressionTest(TestCase):  
    174179        qs = ResolveThis.objects.defer('num')
    175180        self.assertEqual(1, qs.count())
    176181        self.assertEqual('Foobar', qs[0].name)
     182
     183    def test_defer_with_select_related(self):
     184        item1 = Item.objects.create(name="first", value=47)
     185        item2 = Item.objects.create(name="second", value=42)
     186        simple = SimpleItem.objects.create(name="simple", value="23")
     187        related = ItemAndSimpleItem.objects.create(item=item1, simple=simple)
     188
     189        obj = ItemAndSimpleItem.objects.defer('item').select_related('simple').get()
     190        self.assertEqual(obj.item, item1)
     191        self.assertEqual(obj.item_id, item1.id)
     192
     193        obj.item = item2
     194        obj.save()
     195
     196        obj = ItemAndSimpleItem.objects.defer('item').select_related('simple').get()
     197        self.assertEqual(obj.item, item2)
     198        self.assertEqual(obj.item_id, item2.id)

  • tests/regressiontests/select_related_regress/tests.py 

    diff --git a/tests/regressiontests/select_related_regress/tests.py b/tests/regressiontests/select_related_regress/tests.py
index 4cd4f78..3016206 100644
    a b class SelectRelatedRegressTests(TestCase):  
    133133        self.assertEqual(troy.state.name, u'Western Australia')
    134134
    135135        # Also works if you use only, rather than defer
    136         troy = SpecialClient.objects.select_related('state').only('name').get(name='Troy Buswell')
     136        troy = SpecialClient.objects.select_related('state').only('name', 'state').get(name='Troy Buswell')
    137137
    138138        self.assertEqual(troy.name, u'Troy Buswell')
    139139        self.assertEqual(troy.value, 42)
Back to Top