Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #21366 -- regression in join promotion logic

The regression was caused by ecaba36
and affected OR connected filters.
  • Loading branch information...
commit b44d42be6d05e88071844b64c6519223cdd2fa80 1 parent 88b9d4f
Anssi Kääriäinen authored November 02, 2013
12  django/db/models/sql/query.py
@@ -740,7 +740,7 @@ def reset_refcounts(self, to_counts):
740 740
             self.unref_alias(alias, unref_amount)
741 741
 
742 742
     def promote_disjunction(self, aliases_before, alias_usage_counts,
743  
-                            num_childs):
  743
+                            num_childs, left_joins_before):
744 744
         """
745 745
         This method is to be used for promoting joins in ORed filters.
746 746
 
@@ -749,7 +749,8 @@ def promote_disjunction(self, aliases_before, alias_usage_counts,
749 749
         and isn't pre-existing needs to be promoted to LOUTER join.
750 750
         """
751 751
         for alias, use_count in alias_usage_counts.items():
752  
-            if use_count < num_childs and alias not in aliases_before:
  752
+            if ((use_count < num_childs and alias not in aliases_before)
  753
+                    or alias in left_joins_before):
753 754
                 self.promote_joins([alias])
754 755
 
755 756
     def change_aliases(self, change_map):
@@ -1314,9 +1315,14 @@ def _add_q(self, q_object, used_aliases, branch_negated=False,
1314 1315
         if connector == OR:
1315 1316
             alias_usage_counts = dict()
1316 1317
             aliases_before = set(self.tables)
  1318
+            left_joins_before = set()
1317 1319
         for child in q_object.children:
1318 1320
             if connector == OR:
1319 1321
                 refcounts_before = self.alias_refcount.copy()
  1322
+                left_joins_before = left_joins_before.union(set(
  1323
+                    t for t in self.alias_map
  1324
+                    if self.alias_map[t].join_type == self.LOUTER and
  1325
+                    self.alias_refcount[t] > 0))
1320 1326
             if isinstance(child, Node):
1321 1327
                 child_clause = self._add_q(
1322 1328
                     child, used_aliases, branch_negated,
@@ -1332,7 +1338,7 @@ def _add_q(self, q_object, used_aliases, branch_negated=False,
1332 1338
                     alias_usage_counts[alias] = alias_usage_counts.get(alias, 0) + 1
1333 1339
         if connector == OR:
1334 1340
             self.promote_disjunction(aliases_before, alias_usage_counts,
1335  
-                                     len(q_object.children))
  1341
+                                     len(q_object.children), left_joins_before)
1336 1342
         return target_clause
1337 1343
 
1338 1344
     def names_to_path(self, names, opts, allow_many):
19  tests/queries/tests.py
@@ -2719,6 +2719,23 @@ def test_null_join_demotion(self):
2719 2719
         qs = ModelA.objects.filter(Q(b__name__isnull=True) | Q(b__name__isnull=False))
2720 2720
         self.assertTrue(' LEFT OUTER JOIN ' in str(qs.query))
2721 2721
 
  2722
+    def test_ticket_21366(self):
  2723
+        n = Note.objects.create(note='n', misc='m')
  2724
+        e = ExtraInfo.objects.create(info='info', note=n)
  2725
+        a = Author.objects.create(name='Author1', num=1, extra=e)
  2726
+        Ranking.objects.create(rank=1, author=a)
  2727
+        r1 = Report.objects.create(name='Foo', creator=a)
  2728
+        r2 = Report.objects.create(name='Bar')
  2729
+        Report.objects.create(name='Bar', creator=a)
  2730
+        qs = Report.objects.filter(
  2731
+            Q(creator__ranking__isnull=True) |
  2732
+            Q(creator__ranking__rank=1, name='Foo')
  2733
+        )
  2734
+        self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 2)
  2735
+        self.assertEqual(str(qs.query).count(' JOIN '), 2)
  2736
+        self.assertQuerysetEqual(
  2737
+            qs.order_by('name'), [r2, r1], lambda x: x)
  2738
+
2722 2739
 class ReverseJoinTrimmingTest(TestCase):
2723 2740
     def test_reverse_trimming(self):
2724 2741
         # Check that we don't accidentally trim reverse joins - we can't know
@@ -2837,7 +2854,6 @@ def test_disjunction_promotion4(self):
2837 2854
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
2838 2855
 
2839 2856
     def test_disjunction_promotion5_demote(self):
2840  
-        # Failure because no join demotion logic for this case.
2841 2857
         qs = BaseA.objects.filter(Q(a=1) | Q(a=2))
2842 2858
         # Note that the above filters on a force the join to an
2843 2859
         # inner join even if it is trimmed.
@@ -2845,6 +2861,7 @@ def test_disjunction_promotion5_demote(self):
2845 2861
         qs = qs.filter(Q(a__f1='foo') | Q(b__f1='foo'))
2846 2862
         # So, now the a__f1 join doesn't need promotion.
2847 2863
         self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
  2864
+        # But b__f1 does.
2848 2865
         self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1)
2849 2866
         qs = BaseA.objects.filter(Q(a__f1='foo') | Q(b__f1='foo'))
2850 2867
         # Now the join to a is created as LOUTER

0 notes on commit b44d42b

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