diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index e6a3cd2679..e3e3585a8e 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -631,8 +631,10 @@ class Query(object): # We have to do the same "final join" optimisation as in # add_filter, since the final column might not otherwise be part of # the select set (so we can't order on it). - join = self.alias_map[alias] - if col == join[RHS_JOIN_COL]: + while 1: + join = self.alias_map[alias] + if col != join[RHS_JOIN_COL]: + break self.unref_alias(alias) alias = join[LHS_ALIAS] col = join[LHS_JOIN_COL] @@ -830,6 +832,10 @@ class Query(object): if not always_create: for alias in self.join_map.get(t_ident, ()): if alias not in exclusions: + if lhs_table and not self.alias_refcount[self.alias_map[alias][LHS_ALIAS]]: + # The LHS of this join tuple is no longer part of the + # query, so skip this possibility. + continue self.ref_alias(alias) if promote: self.promote_alias(alias) @@ -989,20 +995,22 @@ class Query(object): col = target.column alias = join_list[-1] - if final > 1: + while final > 1: # An optimization: if the final join is against the same column as # we are comparing against, we can go back one step in the join - # chain and compare against the lhs of the join instead. The result - # (potentially) involves one less table join. + # chain and compare against the lhs of the join instead (and then + # repeat the optimization). The result, potentially, involves less + # table joins. join = self.alias_map[alias] - if col == join[RHS_JOIN_COL]: - self.unref_alias(alias) - alias = join[LHS_ALIAS] - col = join[LHS_JOIN_COL] - join_list = join_list[:-1] - final -= 1 - if final == penultimate: - penultimate = last.pop() + if col != join[RHS_JOIN_COL]: + break + self.unref_alias(alias) + alias = join[LHS_ALIAS] + col = join[LHS_JOIN_COL] + join_list = join_list[:-1] + final -= 1 + if final == penultimate: + penultimate = last.pop() if (lookup_type == 'isnull' and value is True and not negate and final > 1): diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py index 5af48741d6..2e84d89ea8 100644 --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -135,6 +135,24 @@ class ManagedModel(models.Model): def __unicode__(self): return self.data +# An inter-related setup with multiple paths from Child to Detail. +class Detail(models.Model): + data = models.CharField(max_length=10) + +class MemberManager(models.Manager): + def get_query_set(self): + return super(MemberManager, self).get_query_set().select_related("details") + +class Member(models.Model): + name = models.CharField(max_length=10) + details = models.OneToOneField(Detail, primary_key=True) + + objects = MemberManager() + +class Child(models.Model): + person = models.OneToOneField(Member, primary_key=True) + parent = models.ForeignKey(Member, related_name="children") + __test__ = {'API_TESTS':""" >>> t1 = Tag(name='t1') @@ -720,5 +738,23 @@ appropriately. >>> Report.objects.values_list("creator__extra__info", flat=True).order_by("name") [u'e1', u'e2', None] +Similarly for select_related(), joins beyond an initial nullable join must +use outer joins so that all results are included. +>>> Report.objects.select_related("creator", "creator__extra").order_by("name") +[, , ] + +When there are multiple paths to a table from another table, we have to be +careful not to accidentally reuse an inappropriate join when using +select_related(). We used to return the parent's Detail record here by mistake. + +>>> d1 = Detail.objects.create(data="d1") +>>> d2 = Detail.objects.create(data="d2") +>>> m1 = Member.objects.create(name="m1", details=d1) +>>> m2 = Member.objects.create(name="m2", details=d2) +>>> c1 = Child.objects.create(person=m2, parent=m1) +>>> obj = m1.children.select_related("person__details")[0] +>>> obj.person.details.data +u'd2' + """}