Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #17886 -- Fixed join promotion in ORed nullable queries

The ORM generated a query with INNER JOIN instead of LEFT OUTER JOIN
in a somewhat complicated case. The main issue was that there was a
chain of nullable FK -> non-nullble FK, and the join promotion logic
didn't see the need to promote the non-nullable FK even if the
previous nullable FK was already promoted to LOUTER JOIN. This resulted
in a query like a LOUTER b INNER c, which incorrectly prunes results.
  • Loading branch information...
commit a193372753ad9d1d15ad5e2d6d06bbc07ca3f433 1 parent fd58d6c
Anssi Kääriäinen authored August 21, 2012
7  django/db/models/sql/query.py
@@ -910,7 +910,12 @@ def join(self, connection, always_create=False, exclusions=(),
910 910
             # Not all tables need to be joined to anything. No join type
911 911
             # means the later columns are ignored.
912 912
             join_type = None
913  
-        elif promote or outer_if_first:
  913
+        elif (promote or outer_if_first
  914
+              or self.alias_map[lhs].join_type == self.LOUTER):
  915
+            # We need to use LOUTER join if asked by promote or outer_if_first,
  916
+            # or if the LHS table is left-joined in the query. Adding inner join
  917
+            # to an existing outer join effectively cancels the effect of the
  918
+            # outer join.
914 919
             join_type = self.LOUTER
915 920
         else:
916 921
             join_type = self.INNER
15  tests/regressiontests/queries/models.py
@@ -385,3 +385,18 @@ class NullableName(models.Model):
385 385
 
386 386
     class Meta:
387 387
         ordering = ['id']
  388
+
  389
+class ModelD(models.Model):
  390
+    name = models.TextField()
  391
+
  392
+class ModelC(models.Model):
  393
+    name = models.TextField()
  394
+
  395
+class ModelB(models.Model):
  396
+    name = models.TextField()
  397
+    c = models.ForeignKey(ModelC)
  398
+
  399
+class ModelA(models.Model):
  400
+    name = models.TextField()
  401
+    b = models.ForeignKey(ModelB, null=True)
  402
+    d = models.ForeignKey(ModelD)
25  tests/regressiontests/queries/tests.py
@@ -23,7 +23,7 @@
23 23
     Ranking, Related, Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten,
24 24
     Node, ObjectA, ObjectB, ObjectC, CategoryItem, SimpleCategory,
25 25
     SpecialCategory, OneToOneCategory, NullableName, ProxyCategory,
26  
-    SingleObject, RelatedObject)
  26
+    SingleObject, RelatedObject, ModelA, ModelD)
27 27
 
28 28
 
29 29
 class BaseQuerysetTest(TestCase):
@@ -2105,3 +2105,26 @@ def test_empty_nodes(self):
2105 2105
         self.assertEqual(w.as_sql(qn, connection), (None, []))
2106 2106
         w = WhereNode(children=[empty_w, NothingNode()], connector='OR')
2107 2107
         self.assertRaises(EmptyResultSet, w.as_sql, qn, connection)
  2108
+
  2109
+class NullJoinPromotionOrTest(TestCase):
  2110
+    def setUp(self):
  2111
+        d = ModelD.objects.create(name='foo')
  2112
+        ModelA.objects.create(name='bar', d=d)
  2113
+
  2114
+    def test_ticket_17886(self):
  2115
+        # The first Q-object is generating the match, the rest of the filters
  2116
+        # should not remove the match even if they do not match anything. The
  2117
+        # problem here was that b__name generates a LOUTER JOIN, then
  2118
+        # b__c__name generates join to c, which the ORM tried to promote but
  2119
+        # failed as that join isn't nullable.
  2120
+        q_obj =  (
  2121
+            Q(d__name='foo')|
  2122
+            Q(b__name='foo')|
  2123
+            Q(b__c__name='foo')
  2124
+        )
  2125
+        qset = ModelA.objects.filter(q_obj)
  2126
+        self.assertEqual(len(qset), 1)
  2127
+        # We generate one INNER JOIN to D. The join is direct and not nullable
  2128
+        # so we can use INNER JOIN for it. However, we can NOT use INNER JOIN
  2129
+        # for the b->c join, as a->b is nullable.
  2130
+        self.assertEqual(str(qset.query).count('INNER JOIN'), 1)

0 notes on commit a193372

Please sign in to comment.
Something went wrong with that request. Please try again.