Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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
@akaariai authored
View
7 django/db/models/sql/query.py
@@ -910,7 +910,12 @@ def join(self, connection, always_create=False, exclusions=(),
# Not all tables need to be joined to anything. No join type
# means the later columns are ignored.
join_type = None
- elif promote or outer_if_first:
+ elif (promote or outer_if_first
+ or self.alias_map[lhs].join_type == self.LOUTER):
+ # We need to use LOUTER join if asked by promote or outer_if_first,
+ # or if the LHS table is left-joined in the query. Adding inner join
+ # to an existing outer join effectively cancels the effect of the
+ # outer join.
join_type = self.LOUTER
else:
join_type = self.INNER
View
15 tests/regressiontests/queries/models.py
@@ -385,3 +385,18 @@ class NullableName(models.Model):
class Meta:
ordering = ['id']
+
+class ModelD(models.Model):
+ name = models.TextField()
+
+class ModelC(models.Model):
+ name = models.TextField()
+
+class ModelB(models.Model):
+ name = models.TextField()
+ c = models.ForeignKey(ModelC)
+
+class ModelA(models.Model):
+ name = models.TextField()
+ b = models.ForeignKey(ModelB, null=True)
+ d = models.ForeignKey(ModelD)
View
25 tests/regressiontests/queries/tests.py
@@ -23,7 +23,7 @@
Ranking, Related, Report, ReservedName, Tag, TvChef, Valid, X, Food, Eaten,
Node, ObjectA, ObjectB, ObjectC, CategoryItem, SimpleCategory,
SpecialCategory, OneToOneCategory, NullableName, ProxyCategory,
- SingleObject, RelatedObject)
+ SingleObject, RelatedObject, ModelA, ModelD)
class BaseQuerysetTest(TestCase):
@@ -2105,3 +2105,26 @@ def test_empty_nodes(self):
self.assertEqual(w.as_sql(qn, connection), (None, []))
w = WhereNode(children=[empty_w, NothingNode()], connector='OR')
self.assertRaises(EmptyResultSet, w.as_sql, qn, connection)
+
+class NullJoinPromotionOrTest(TestCase):
+ def setUp(self):
+ d = ModelD.objects.create(name='foo')
+ ModelA.objects.create(name='bar', d=d)
+
+ def test_ticket_17886(self):
+ # The first Q-object is generating the match, the rest of the filters
+ # should not remove the match even if they do not match anything. The
+ # problem here was that b__name generates a LOUTER JOIN, then
+ # b__c__name generates join to c, which the ORM tried to promote but
+ # failed as that join isn't nullable.
+ q_obj = (
+ Q(d__name='foo')|
+ Q(b__name='foo')|
+ Q(b__c__name='foo')
+ )
+ qset = ModelA.objects.filter(q_obj)
+ self.assertEqual(len(qset), 1)
+ # We generate one INNER JOIN to D. The join is direct and not nullable
+ # so we can use INNER JOIN for it. However, we can NOT use INNER JOIN
+ # for the b->c join, as a->b is nullable.
+ self.assertEqual(str(qset.query).count('INNER JOIN'), 1)
Please sign in to comment.
Something went wrong with that request. Please try again.