Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #20528 -- regression in select_related join promotion

The join used by select_related was incorrectly INNER when the query
had an ORed filter for nullable join that was trimmed away. Fixed this
by forcing the join type to LOUTER even when a join was trimmed away
in ORed queries.
  • Loading branch information...
commit 89bf7a45250cf554295f3d17d708311d3edba101 1 parent b7bd708
@akaariai akaariai authored
Showing with 35 additions and 18 deletions.
  1. +3 −1 django/db/models/sql/query.py
  2. +32 −17 tests/queries/tests.py
View
4 django/db/models/sql/query.py
@@ -1913,5 +1913,7 @@ def alias_diff(refcounts_before, refcounts_after):
Given the before and after copies of refcounts works out which aliases
have been added to the after copy.
"""
+ # Use -1 as default value so that any join that is created, then trimmed
+ # is seen as added.
return set(t for t in refcounts_after
- if refcounts_after[t] > refcounts_before.get(t, 0))
+ if refcounts_after[t] > refcounts_before.get(t, -1))
View
49 tests/queries/tests.py
@@ -16,15 +16,16 @@
from django.utils import unittest
from django.utils.datastructures import SortedDict
-from .models import (Annotation, Article, Author, Celebrity, Child, Cover,
- Detail, DumbCategory, ExtraInfo, Fan, Item, LeafA, Join, LeafB, LoopX, LoopZ,
- ManagedModel, Member, NamedCategory, Note, Number, Plaything, PointerA,
- Ranking, Related, Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten,
- Node, ObjectA, ObjectB, ObjectC, CategoryItem, SimpleCategory,
- SpecialCategory, OneToOneCategory, NullableName, ProxyCategory,
- SingleObject, RelatedObject, ModelA, ModelB, ModelC, ModelD, Responsibility,
- Job, JobResponsibilities, BaseA, Identifier, Program, Channel, Page,
- Paragraph, Chapter, Book, MyObject, Order, OrderItem)
+from .models import (
+ Annotation, Article, Author, Celebrity, Child, Cover, Detail, DumbCategory,
+ ExtraInfo, Fan, Item, LeafA, Join, LeafB, LoopX, LoopZ, ManagedModel,
+ Member, NamedCategory, Note, Number, Plaything, PointerA, Ranking, Related,
+ Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten, Node, ObjectA,
+ ObjectB, ObjectC, CategoryItem, SimpleCategory, SpecialCategory,
+ OneToOneCategory, NullableName, ProxyCategory, SingleObject, RelatedObject,
+ ModelA, ModelB, ModelC, ModelD, Responsibility, Job, JobResponsibilities,
+ BaseA, FK1, Identifier, Program, Channel, Page, Paragraph, Chapter, Book,
+ MyObject, Order, OrderItem)
class BaseQuerysetTest(TestCase):
@@ -2620,6 +2621,19 @@ def test_revfk_noreuse(self):
self.assertEqual(str(qs.query).count('JOIN'), 2)
class DisjunctionPromotionTests(TestCase):
+ def test_disjuction_promotion_select_related(self):
+ fk1 = FK1.objects.create(f1='f1', f2='f2')
+ basea = BaseA.objects.create(a=fk1)
+ qs = BaseA.objects.filter(Q(a=fk1) | Q(b=2))
+ self.assertEqual(str(qs.query).count(' JOIN '), 0)
+ qs = qs.select_related('a', 'b')
+ self.assertEqual(str(qs.query).count(' INNER JOIN '), 0)
+ self.assertEqual(str(qs.query).count(' LEFT OUTER JOIN '), 2)
+ with self.assertNumQueries(1):
+ self.assertQuerysetEqual(qs, [basea], lambda x: x)
+ self.assertEqual(qs[0].a, fk1)
+ self.assertIs(qs[0].b, None)
+
def test_disjunction_promotion1(self):
# Pre-existing join, add two ORed filters to the same join,
# all joins can be INNER JOINS.
@@ -2669,17 +2683,23 @@ def test_disjunction_promotion3_failing(self):
self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1)
- def test_disjunction_promotion4(self):
+ @unittest.expectedFailure
+ def test_disjunction_promotion4_failing(self):
+ # Failure because no join repromotion
qs = BaseA.objects.filter(Q(a=1) | Q(a=2))
self.assertEqual(str(qs.query).count('JOIN'), 0)
qs = qs.filter(a__f1='foo')
self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
+
+ def test_disjunction_promotion4(self):
qs = BaseA.objects.filter(a__f1='foo')
self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
qs = qs.filter(Q(a=1) | Q(a=2))
self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
- def test_disjunction_promotion5(self):
+ @unittest.expectedFailure
+ def test_disjunction_promotion5_failing(self):
+ # Failure because no join repromotion logic.
qs = BaseA.objects.filter(Q(a=1) | Q(a=2))
# Note that the above filters on a force the join to an
# inner join even if it is trimmed.
@@ -2688,15 +2708,10 @@ def test_disjunction_promotion5(self):
# So, now the a__f1 join doesn't need promotion.
self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1)
-
- @unittest.expectedFailure
- def test_disjunction_promotion5_failing(self):
qs = BaseA.objects.filter(Q(a__f1='foo') | Q(b__f1='foo'))
# Now the join to a is created as LOUTER
self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 0)
- # The below filter should force the a to be inner joined. But,
- # this is failing as we do not have join unpromotion logic.
- qs = BaseA.objects.filter(Q(a=1) | Q(a=2))
+ qs = qs.objects.filter(Q(a=1) | Q(a=2))
self.assertEqual(str(qs.query).count('INNER JOIN'), 1)
self.assertEqual(str(qs.query).count('LEFT OUTER JOIN'), 1)
Please sign in to comment.
Something went wrong with that request. Please try again.