Ticket #14056: 14056_join_by_reverse_relation.diff

django/db/models/sql/compiler.py

@@ -437 +437 @@
             if not self.query.alias_refcount[alias]:
                 continue
             try:
-                name, alias, join_type, lhs, lhs_col, col, nullable = self.query.alias_map[alias]
+                (name, alias, join_type, lhs,
+                        lhs_col, col, nullable, direct) = self.query.alias_map[alias]
             except KeyError:
                 # Extra tables can end up in self.tables, but not in the
                 # alias_map if they aren't in a join. That's OK. We skip them.

django/db/models/sql/constants.py

@@ -24 +24 @@
 LHS_JOIN_COL = 4
 RHS_JOIN_COL = 5
 NULLABLE = 6
+DIRECT = 7
 
 # How many results to expect from a cursor.execute call
 MULTI = 'multi'

django/db/models/sql/query.py

@@ -806 +806 @@
         return len([1 for count in self.alias_refcount.itervalues() if count])
 
     def join(self, connection, always_create=False, exclusions=(),
-            promote=False, outer_if_first=False, nullable=False, reuse=None):
+            promote=False, outer_if_first=False, nullable=False, reuse=None,
+            direct=True):
         """
         Returns an alias for the join in 'connection', either reusing an
         existing alias for that join or creating a new one. 'connection' is a
@@ -837 +838 @@
 
         If 'nullable' is True, the join can potentially involve NULL values and
         is a candidate for promotion (to "left outer") when combining querysets.
+
+        If 'direct' is False, the join were got by reverse relation.
         """
         lhs, table, lhs_col, col = connection
         if lhs in self.alias_map:
@@ -874 +877 @@
             join_type = self.LOUTER
         else:
             join_type = self.INNER
-        join = (table, alias, join_type, lhs, lhs_col, col, nullable)
+        join = (table, alias, join_type, lhs, lhs_col, col, nullable, direct)
         self.alias_map[alias] = join
         if t_ident in self.join_map:
             self.join_map[t_ident] += (alias,)
@@ -1286 +1289 @@
                         joins.extend([int_alias, alias])
                 elif field.rel:
                     # One-to-one or many-to-one field
+                    # (ForeignKey defined on the current model)
                     if cached_data:
                         (table, from_col, to_col, opts, target) = cached_data
                     else:
@@ -1333 +1337 @@
                             reuse=can_reuse)
                     joins.extend([int_alias, alias])
                 else:
-                    # One-to-many field (ForeignKey defined on the target model)
+                    # One-to-one or one-to-many fields
+                    # (ForeignKey defined on the target model)
                     if cached_data:
                         (table, from_col, to_col, opts, target) = cached_data
                     else:
@@ -1349 +1354 @@
 
                     alias = self.join((alias, table, from_col, to_col),
                             dupe_multis, exclusions, nullable=True,
-                            reuse=can_reuse)
+                            reuse=can_reuse, direct=False)
                     joins.append(alias)
 
             for (dupe_opts, dupe_col) in dupe_set:
@@ -1407 +1412 @@
         alias = join_list[-1]
         while final > 1:
             join = self.alias_map[alias]
-            if col != join[RHS_JOIN_COL] or join[JOIN_TYPE] != self.INNER:
+            if col != join[RHS_JOIN_COL] or join[JOIN_TYPE] != self.INNER or \
+               not join[DIRECT]:
                 break
             self.unref_alias(alias)
             alias = join[LHS_ALIAS]

tests/regressiontests/many_to_one_regress/tests.py

@@ -1 +1 @@
 from django.db import models
 from django.test import TestCase
 
-from models import First, Second, Third, Parent, Child, Category, Record, Relation
+from models import (First, Second, Third, Parent, Child, 
+                    Category, Record, Relation, Place, Restaurant)
 
 class ManyToOneRegressionTests(TestCase):
     def test_object_creation(self):
@@ -103 +104 @@
         # of a model, and interrogate its related field.
         cat = models.ForeignKey(Category)
         self.assertEqual('id', cat.rel.get_related_field().name)
+
+    def test_reverse_many_to_one(self):
+        """
+        Regression test for #14056
+
+        Reverse lookups generated joins which have been optimized incorrectly.
+        """
+        p1 = Place.objects.create(name='Demon Dogs')
+        r1 = Restaurant.objects.create(place=p1)
+        self.assertQuerysetEqual(
+                Place.objects.filter(restaurant__place=p1),
+                ['<Place: Demon Dogs the place>']
+        )
+
+        p2 = Place.objects.create(name='Cute Cats')
+        self.assertQuerysetEqual(
+                Place.objects.filter(restaurant__place=p2),
+                []
+        )

tests/regressiontests/many_to_one_regress/models.py

@@ -44 +44 @@
 
     def __unicode__(self):
         return u"%s - %s" % (self.left.category.name, self.right.category.name)
+
+# Reverse lookup (#14056)
+class Place(models.Model):
+    name = models.CharField(max_length=50)
+
+    def __unicode__(self):
+        return u"%s the place" % self.name
+
+class Restaurant(models.Model):
+    place = models.ForeignKey(Place)

tests/regressiontests/one_to_one_regress/tests.py

@@ -128 +128 @@
                 Target.objects.exclude(pointer2=None),
                 []
         )
+
+    def test_reverse_one_to_one(self):
+        """
+        Regression test for #14056
+
+        Reverse lookups generated joins which have been optimized incorrectly.
+        """
+        self.assertQuerysetEqual(
+                Place.objects.filter(restaurant__place=self.p1),
+                ['<Place: Demon Dogs the place>']
+        )
+
+        p2 = Place.objects.create(name='Cute Cats', address='123 Street')
+        self.assertQuerysetEqual(
+                Place.objects.filter(restaurant__place=p2),
+                []
+        )