Ticket #10790: 107090v3.diff

django/db/models/sql/compiler.py

@@ -374 +374 @@
         pieces = name.split(LOOKUP_SEP)
         if not alias:
             alias = self.query.get_initial_alias()
-        field, target, opts, joins, last, extra = self.query.setup_joins(pieces,
-                opts, alias, False)
+        field, target, opts, joins, last, extra, allow_trim_join = self.query.setup_joins(
+                pieces, opts, alias, False)
         alias = joins[-1]
         col = target.column
         if not field.rel:

django/db/models/sql/expressions.py

@@ -41 +41 @@
             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, _, allow_trim_join = 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

@@ -651 +651 @@
             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
+        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
@@ -929 +937 @@
             #   - 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
@@ -1021 +1029 @@
         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, can_reuse=can_reuse,
                     negate=negate, process_extras=process_extras)
         except MultiJoin, e:
@@ -1036 +1044 @@
             # needed, as it's less efficient at the database level.
             self.promote_alias_chain(join_list)
 
+        # 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).
         col, alias, join_list = self.trim_joins(target, join_list, last, trim)
@@ -1166 +1181 @@
         dupe_set = set()
         exclusions = set()
         extra_filters = []
+        allow_trim_join = True
         for pos, name in enumerate(names):
             try:
                 exclusions.add(int_alias)
@@ -1190 +1206 @@
                     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)
@@ -1231 +1252 @@
                 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,
@@ -1340 +1363 @@
             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):
         """
@@ -1489 +1512 @@
 
         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]
@@ -1766 +1789 @@
         """
         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/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)
+
+    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)
+
+

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
+