diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index de431204bd..15f2643495 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -480,7 +480,7 @@ class Query(object): 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 @@ class Query(object): """ 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 @@ class Query(object): # 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 @@ class Query(object): # 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] diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index e1a880585f..4d46cae766 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -596,3 +596,24 @@ class BaseAggregateTestCase(TestCase): 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)