From 9282d1d27823e780ccad7ddb006182e27e66262e Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 30 Jan 2012 12:42:57 -0600 Subject: [PATCH] Fixed #13781 -- select_related and multiple inheritance --- django/db/models/query.py | 16 +++++++++--- django/db/models/sql/compiler.py | 12 ++++++--- .../select_related_onetoone/models.py | 26 ++++++++++++++++++++ .../select_related_onetoone/tests.py | 23 ++++++++++++++++- 4 files changed, 68 insertions(+), 9 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 324554e..ede2581 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -1126,7 +1126,8 @@ class EmptyQuerySet(QuerySet): def get_cached_row(klass, row, index_start, using, max_depth=0, cur_depth=0, - requested=None, offset=0, only_load=None, local_only=False): + requested=None, offset=0, only_load=None, local_only=False, + last_klass=None): """ Helper function that recursively returns an object with the specified related attributes already populated. @@ -1156,6 +1157,8 @@ def get_cached_row(klass, row, index_start, using, max_depth=0, cur_depth=0, the full field list for `klass` can be assumed. * local_only - Only populate local fields. This is used when building following reverse select-related relations + * last_klass - the last class seen when following reverse + select-related relations """ if max_depth and requested is None and cur_depth > max_depth: # We've recursed deeply enough; stop now. @@ -1202,7 +1205,11 @@ def get_cached_row(klass, row, index_start, using, max_depth=0, cur_depth=0, else: # Load all fields on klass if local_only: - field_names = [f.attname for f in klass._meta.local_fields] + parents = [p for p in klass._meta.get_parent_list() + if p is not last_klass] + field_names = [f.attname for f in klass._meta.fields + if f in klass._meta.local_fields + or f.model in parents] else: field_names = [f.attname for f in klass._meta.fields] field_count = len(field_names) @@ -1261,7 +1268,8 @@ def get_cached_row(klass, row, index_start, using, max_depth=0, cur_depth=0, next = requested[f.related_query_name()] # Recursively retrieve the data for the related object cached_row = get_cached_row(model, row, index_end, using, - max_depth, cur_depth+1, next, only_load=only_load, local_only=True) + max_depth, cur_depth+1, next, only_load=only_load, local_only=True, + last_klass=klass) # If the recursive descent found an object, populate the # descriptor caches relevant to the object if cached_row: @@ -1277,7 +1285,7 @@ def get_cached_row(klass, row, index_start, using, max_depth=0, cur_depth=0, # Now populate all the non-local field values # on the related object for rel_field,rel_model in rel_obj._meta.get_fields_with_model(): - if rel_model is not None: + if rel_model is not None and isinstance(obj, rel_model): setattr(rel_obj, rel_field.attname, getattr(obj, rel_field.attname)) # populate the field cache for any related object # that has already been retrieved diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index d425c8b..7c0ff75 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -216,7 +216,8 @@ class SQLCompiler(object): return result def get_default_columns(self, with_aliases=False, col_aliases=None, - start_alias=None, opts=None, as_pairs=False, local_only=False): + start_alias=None, opts=None, as_pairs=False, local_only=False, + last_opts=None): """ Computes the default columns for selecting every field in the base model. Will sometimes be called to pull in related models (e.g. via @@ -240,8 +241,9 @@ class SQLCompiler(object): if start_alias: seen = {None: start_alias} + parents = [p for p in opts.get_parent_list() if p._meta is not last_opts] for field, model in opts.get_fields_with_model(): - if local_only and model is not None: + if local_only and model is not None and model not in parents: continue if start_alias: try: @@ -252,7 +254,8 @@ class SQLCompiler(object): else: link_field = opts.get_ancestor_link(model) alias = self.query.join((start_alias, model._meta.db_table, - link_field.column, model._meta.pk.column)) + link_field.column, model._meta.pk.column), + promote=(model in parents)) seen[model] = alias else: # If we're starting from the base model of the queryset, the @@ -650,7 +653,8 @@ class SQLCompiler(object): ) used.add(alias) columns, aliases = self.get_default_columns(start_alias=alias, - opts=model._meta, as_pairs=True, local_only=True) + opts=model._meta, as_pairs=True, local_only=True, + last_opts=opts) self.query.related_select_cols.extend(columns) self.query.related_select_fields.extend(model._meta.fields) diff --git a/tests/regressiontests/select_related_onetoone/models.py b/tests/regressiontests/select_related_onetoone/models.py index 3d6da9b..4bfad1d 100644 --- a/tests/regressiontests/select_related_onetoone/models.py +++ b/tests/regressiontests/select_related_onetoone/models.py @@ -45,6 +45,7 @@ class StatDetails(models.Model): class AdvancedUserStat(UserStat): karma = models.IntegerField() + class Image(models.Model): name = models.CharField(max_length=100) @@ -52,3 +53,28 @@ class Image(models.Model): class Product(models.Model): name = models.CharField(max_length=100) image = models.OneToOneField(Image, null=True) + + +class Parent1(models.Model): + name1 = models.CharField(max_length=50) + def __unicode__(self): + return self.name1 + + +class Parent2(models.Model): + name2 = models.CharField(max_length=50) + def __unicode__(self): + return self.name2 + + +class Child1(Parent1, Parent2): + other = models.CharField(max_length=50) + def __unicode__(self): + return self.name1 + + +class Child2(Parent1): + parent2 = models.OneToOneField(Parent2) + other = models.CharField(max_length=50) + def __unicode__(self): + return self.name1 diff --git a/tests/regressiontests/select_related_onetoone/tests.py b/tests/regressiontests/select_related_onetoone/tests.py index ab35fec..407cdc9 100644 --- a/tests/regressiontests/select_related_onetoone/tests.py +++ b/tests/regressiontests/select_related_onetoone/tests.py @@ -3,7 +3,7 @@ from django.conf import settings from django.test import TestCase from models import (User, UserProfile, UserStat, UserStatResult, StatDetails, - AdvancedUserStat, Image, Product) + AdvancedUserStat, Image, Product, Parent1, Parent2, Child1, Child2) class ReverseSelectRelatedTestCase(TestCase): def setUp(self): @@ -20,6 +20,14 @@ class ReverseSelectRelatedTestCase(TestCase): advstat = AdvancedUserStat.objects.create(user=user2, posts=200, karma=5, results=results2) StatDetails.objects.create(base_stats=advstat, comments=250) + p1 = Parent1(name1="Only Parent1") + p1.save() + c1 = Child1(name1="Child1 Parent1", name2="Child1 Parent2") + c1.save() + p2 = Parent2(name2="Child2 Parent2") + p2.save() + c2 = Child2(name1="Child2 Parent1", parent2=p2) + c2.save() def test_basic(self): def test(): @@ -88,3 +96,16 @@ class ReverseSelectRelatedTestCase(TestCase): p2 = Product.objects.create(name="Talking Django Plushie") self.assertEqual(len(Product.objects.select_related("image")), 2) + + def test_parent_only(self): + Parent1.objects.select_related('child1').get(name1="Only Parent1") + + def test_multiple_subclass(self): + with self.assertNumQueries(1): + p = Parent1.objects.select_related('child1').get(name1="Child1 Parent1") + self.assertEqual(p.child1.name2, u"Child1 Parent2") + + def test_onetoone_with_subclass(self): + with self.assertNumQueries(1): + p = Parent2.objects.select_related('child2').get(name2="Child2 Parent2") + self.assertEqual(p.child2.name1, u"Child2 Parent1") -- 1.7.7.3