Ticket #10790: ticket10790_v5.diff

django/db/models/sql/compiler.py

@@ -456 +456 @@
         """
         if not alias:
             alias = self.query.get_initial_alias()
-        field, target, opts, joins, _, _ = self.query.setup_joins(pieces,
-                opts, alias, False)
+        field, target, opts, joins, _, _, _ = self.query.setup_joins(
+                pieces, opts, alias, False)
         alias = joins[-1]
         col = target.column
         if not field.rel:
@@ -511 +511 @@
             if not self.query.alias_refcount[alias]:
                 continue
             try:
-                name, alias, join_type, lhs, lhs_col, col, nullable = self.query.alias_map[alias]
+                name, alias, join_type, lhs, lhs_col, col, nullable, demoted_join = self.query.alias_map[alias]
             except KeyError:
                 # Extra tables can end up in self.tables, but not in the
                 # alias_map if they aren't in a join. That's OK. We skip them.

django/db/models/sql/constants.py

@@ -24 +24 @@
 LHS_JOIN_COL = 4
 RHS_JOIN_COL = 5
 NULLABLE = 6
+DEMOTED_JOIN = 7
 
 # How many results to expect from a cursor.execute call
 MULTI = 'multi'

django/db/models/sql/expressions.py

@@ -44 +44 @@
             self.cols[node] = query.aggregate_select[node.name]
         else:
             try:
-                field, source, opts, join_list, last, _ = query.setup_joins(
+                field, source, opts, join_list, last, _, _ = query.setup_joins(
                     field_list, query.get_meta(),
                     query.get_initial_alias(), False)
                 col, _, join_list = query.trim_joins(source, join_list, last, False)

django/db/models/sql/query.py

@@ -465 +465 @@
         conjunction = (connector == AND)
         first = True
         for alias in rhs.tables:
-            if not rhs.alias_refcount[alias]:
+            demoted_alias = rhs.alias_map[alias][DEMOTED_JOIN]
+            # the alias can be ignored only if it was not demoted due to the fkey trim join
+            if not rhs.alias_refcount[alias] and not demoted_alias:
                 # An unused alias.
                 continue
-            promote = (rhs.alias_map[alias][JOIN_TYPE] == self.LOUTER)
+            promote = (rhs.alias_map[alias][JOIN_TYPE] == self.LOUTER or demoted_alias)
             lhs, table, lhs_col, col = rhs.rev_join_map[alias]
             # If the left side of the join was already relabeled, use the
             # updated alias.
@@ -700 +702 @@
             return True
         return False
 
+    def demote_alias(self, alias):
+        """
+        Demotes the join type of an alias to an inner join.
+        """
+        data = list(self.alias_map[alias])
+        data[JOIN_TYPE] = self.INNER
+        data[DEMOTED_JOIN] = True
+        self.alias_map[alias] = tuple(data)
+
     def promote_alias_chain(self, chain, must_promote=False):
         """
         Walks along a chain of aliases, promoting the first nullable join and
@@ -906 +917 @@
                     if self.alias_map[alias][LHS_ALIAS] != lhs:
                         continue
                     self.ref_alias(alias)
-                    if promote:
+                    if promote or self.alias_map[alias][DEMOTED_JOIN]:
                         self.promote_alias(alias)
                     return alias
 
         # No reuse is possible, so we need a new alias.
         alias, _ = self.table_alias(table, True)
+
         if not lhs:
             # 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:
             join_type = self.LOUTER
+        elif lhs in self.alias_map and self.alias_map[lhs][DEMOTED_JOIN]:
+            # if the lhs is already present in the alias_map
+            # and its join was DEMOTED earlier, change its join type to LOUTER
+            # and promote LOUTER join
+            if self.alias_map[lhs][JOIN_TYPE] == self.INNER:
+                self.promote_alias(lhs, unconditional=True)
+            join_type = self.LOUTER
         else:
             join_type = self.INNER
-        join = (table, alias, join_type, lhs, lhs_col, col, nullable)
+        join = (table, alias, join_type, lhs, lhs_col, col, nullable, False)
         self.alias_map[alias] = join
         if t_ident in self.join_map:
             self.join_map[t_ident] += (alias,)
@@ -1007 +1026 @@
             #   - this is an annotation over a model field
             # then we need to explore the joins that are required.
 
-            field, source, opts, join_list, last, _ = self.setup_joins(
+            field, source, opts, join_list, last, _, allow_trim_join = self.setup_joins(
                 field_list, opts, self.get_initial_alias(), False)
 
             # Process the join chain to see if it can be trimmed
@@ -1119 +1138 @@
         allow_many = trim or not negate
 
         try:
-            field, target, opts, join_list, last, extra_filters = self.setup_joins(
+            field, target, opts, join_list, last, extra_filters, allow_trim_join = self.setup_joins(
                     parts, opts, alias, True, allow_many, allow_explicit_fk=True,
                     can_reuse=can_reuse, negate=negate,
                     process_extras=process_extras)
@@ -1139 +1158 @@
             self.promote_alias_chain(join_list)
             join_promote = True
 
+            # If we have a one2one or many2one field, we can trim the left outer
+            # join from the end of a list of joins.
+            # In order to do this, we convert alias join type back to INNER and
+            # trim_joins later will do the strip for us.
+            if allow_trim_join and field.rel:
+                self.demote_alias(join_list[-1])
+
         # Process the join list to see if we can remove any inner joins from
         # the far end (fewer tables in a query is better).
         nonnull_comparison = (lookup_type == 'isnull' and value is False)
@@ -1295 +1321 @@
         dupe_set = set()
         exclusions = set()
         extra_filters = []
+        allow_trim_join = True
         int_alias = None
         for pos, name in enumerate(names):
             if int_alias is not None:
@@ -1318 +1345 @@
                     raise FieldError("Cannot resolve keyword %r into field. "
                             "Choices are: %s" % (name, ", ".join(names)))
 
+            # presence of indirect field in the filter requires
+            # left outer join for isnull
+            if not direct and allow_trim_join:
+                allow_trim_join = False
+
             if not allow_many and (m2m or not direct):
                 for alias in joins:
                     self.unref_alias(alias)
@@ -1359 +1391 @@
                 extra_filters.extend(field.extra_filters(names, pos, negate))
             if direct:
                 if m2m:
+                    # null query on m2mfield requires outer join
+                    allow_trim_join = False
                     # Many-to-many field defined on the current model.
                     if cached_data:
                         (table1, from_col1, to_col1, table2, from_col2,
@@ -1479 +1513 @@
             else:
                 raise FieldError("Join on field %r not permitted." % name)
 
-        return field, target, opts, joins, last, extra_filters
+        return field, target, opts, joins, last, extra_filters, allow_trim_join
 
     def trim_joins(self, target, join_list, last, trim, nonnull_check=False):
         """
@@ -1648 +1682 @@
 
         try:
             for name in field_names:
-                field, target, u2, joins, u3, u4 = self.setup_joins(
+                field, target, u2, joins, u3, u4, allow_trim_join = self.setup_joins(
                         name.split(LOOKUP_SEP), opts, alias, False, allow_m2m,
                         True)
                 final_alias = joins[-1]
@@ -1930 +1964 @@
         """
         opts = self.model._meta
         alias = self.get_initial_alias()
-        field, col, opts, joins, last, extra = self.setup_joins(
+        field, col, opts, joins, last, extra, allow_trim_join = self.setup_joins(
                 start.split(LOOKUP_SEP), opts, alias, False)
         select_col = self.alias_map[joins[1]][LHS_JOIN_COL]
         select_alias = alias

tests/modeltests/null_trimjoin/__init__.py

@@ +1 @@
+# dummy text for patch

tests/modeltests/null_trimjoin/models.py

@@ +1 @@
+"""
+Do not join table when querying on isnull
+
+"""
+
+from django.db import models
+
+class Category(models.Model):
+    name = models.CharField(max_length=30)
+
+class ReporterType(models.Model):
+    name = models.CharField(max_length=30)
+
+class Reporter(models.Model):
+    name = models.CharField(max_length=30)
+    type = models.ForeignKey(ReporterType, null=True)
+    category = models.ManyToManyField(Category, null=True)
+
+    def __unicode__(self):
+        return self.name
+
+class Article(models.Model):
+    headline = models.CharField(max_length=100)
+    reporter = models.ForeignKey(Reporter, null=True)
+
+    class Meta:
+        ordering = ('headline',)
+
+    def __unicode__(self):
+        return self.headline
+

tests/modeltests/null_trimjoin/tests.py

@@ +1 @@
+from django.test import TestCase
+from models import Article, Reporter
+
+class OneToOneTests(TestCase):
+
+    def setUp(self):
+        self.r = Reporter(name='John Smith')
+        self.r.save()
+        self.a = Article(headline="First", reporter=self.r)
+        self.a.save()
+        self.a2 = Article(headline="Second")
+        self.a2.save()
+
+    def test_query_with_isnull(self):
+        """Querying with isnull should not join Reporter table."""
+        q = Article.objects.filter(reporter=None)
+        # check that reporter is not in the query's used_aliases
+        self.assertFalse('null_trimjoin_reporter' in q.query.used_aliases)
+        self.assertTrue('null_trimjoin_article' in q.query.used_aliases)
+        # but it should still be in query.tables
+        self.assertTrue('null_trimjoin_article' in q.query.tables)
+        self.assertTrue('null_trimjoin_reporter' in q.query.tables)
+
+    def test_query_across_tables(self):
+        """Querying across several tables should strip only the last join, while 
+        preserving the preceding left outer joins."""
+        q = Article.objects.filter(reporter__type=None)
+        self.assertEquals(len(q), 2)
+        self.assertTrue('null_trimjoin_article' in q.query.used_aliases)
+        self.assertTrue('null_trimjoin_reporter' in q.query.used_aliases)
+        self.assertFalse('null_trimjoin_reportertype' in q.query.used_aliases)
+
+    def test_m2m_query(self):
+        """Querying across m2m field should not strip the m2m table from join."""
+        q = Article.objects.filter(reporter__category__isnull=True)
+        self.assertTrue('null_trimjoin_article' in q.query.used_aliases)
+        self.assertTrue('null_trimjoin_reporter' in q.query.used_aliases)
+        self.assertTrue('null_trimjoin_category' in q.query.used_aliases)
+
+    def test_reverse_query(self):
+        """Reverse querying with isnull should not strip the join."""
+        q = Reporter.objects.filter(article__isnull=True)
+        self.assertTrue('null_trimjoin_reporter' in q.query.used_aliases)
+
+