Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #17424 -- annotate() + exclude() bug

The bug was already fixed by 01b9c3d,
so only tests added.

At the same time promote_joins()'s uncoditional flag is gone, it isn't
needed for anything any more.
  • Loading branch information...
commit c7739e30b20f55c2b055b12a628bfb5c2228ba4e 1 parent d53e574
@akaariai akaariai authored
Showing with 28 additions and 5 deletions.
  1. +7 −5 django/db/models/sql/query.py
  2. +21 −0 tests/aggregation/tests.py
View
12 django/db/models/sql/query.py
@@ -480,7 +480,7 @@ def combine(self, rhs, connector):
rhs_used_joins = set(change_map.values())
to_promote = [alias for alias in self.tables
if alias not in rhs_used_joins]
- self.promote_joins(to_promote, True)
+ self.promote_joins(to_promote)
# Now relabel a copy of the rhs where-clause and add it to the current
# one.
@@ -659,7 +659,7 @@ def unref_alias(self, alias, amount=1):
""" Decreases the reference count for this alias. """
self.alias_refcount[alias] -= amount
- def promote_joins(self, aliases, unconditional=False):
+ def promote_joins(self, aliases):
"""
Promotes recursively the join type of given aliases and its children to
an outer join. If 'unconditional' is False, the join is only promoted if
@@ -682,12 +682,14 @@ def promote_joins(self, aliases, unconditional=False):
# isn't really joined at all in the query, so we should not
# alter its join type.
continue
+ # Only the first alias (skipped above) should have None join_type
+ assert self.alias_map[alias].join_type is not None
parent_alias = self.alias_map[alias].lhs_alias
parent_louter = (parent_alias
and self.alias_map[parent_alias].join_type == self.LOUTER)
already_louter = self.alias_map[alias].join_type == self.LOUTER
- if ((unconditional or self.alias_map[alias].nullable
- or parent_louter) and not already_louter):
+ if ((self.alias_map[alias].nullable or parent_louter) and
+ not already_louter):
data = self.alias_map[alias]._replace(join_type=self.LOUTER)
self.alias_map[alias] = data
# Join type of 'alias' changed, so re-examine all aliases that
@@ -986,7 +988,7 @@ def add_aggregate(self, aggregate, model, alias, is_summary):
# If the aggregate references a model or field that requires a join,
# those joins must be LEFT OUTER - empty join rows must be returned
# in order for zeros to be returned for those aggregates.
- self.promote_joins(join_list, True)
+ self.promote_joins(join_list)
col = targets[0].column
source = sources[0]
View
21 tests/aggregation/tests.py
@@ -596,3 +596,24 @@ def test_values_aggregation(self):
self.assertEqual(
max_books_per_rating,
{'books_per_rating__max': 3})
+
+ def test_ticket17424(self):
+ """
+ Check that doing exclude() on a foreign model after annotate()
+ doesn't crash.
+ """
+ all_books = list(Book.objects.values_list('pk', flat=True).order_by('pk'))
+ annotated_books = Book.objects.order_by('pk').annotate(one=Count("id"))
+
+ # The value doesn't matter, we just need any negative
+ # constraint on a related model that's a noop.
+ excluded_books = annotated_books.exclude(publisher__name="__UNLIKELY_VALUE__")
+
+ # Try to generate query tree
+ str(excluded_books.query)
+
+ self.assertQuerysetEqual(excluded_books, all_books, lambda x: x.pk)
+
+ # Check internal state
+ self.assertIsNone(annotated_books.query.alias_map["aggregation_book"].join_type)
+ self.assertIsNone(excluded_books.query.alias_map["aggregation_book"].join_type)
Please sign in to comment.
Something went wrong with that request. Please try again.