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

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

At the same time promote_joins()'s uncoditional flag is gone, it isn't
needed for anything any more.

2 files changed, 28 insertions, 5 deletions

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)