Ticket #10790: ticket10790_v6.diff

File ticket10790_v6.diff, 18.5 KB (added by milosu, 12 years ago)

elaborated patch and test case supporting more isnull use cases, passing Django test suite

  • django/db/models/sql/compiler.py 

    diff -urN Django-1.4b1.orig/django/db/models/sql/compiler.py Django-1.4b1/django/db/models/sql/compiler.py
    old new  
    456456        """
    457457        if not alias:
    458458            alias = self.query.get_initial_alias()
    459         field, target, opts, joins, _, _ = self.query.setup_joins(pieces,
    460                 opts, alias, False)
     459        field, target, opts, joins, _, _, _ = self.query.setup_joins(
     460                pieces, opts, alias, False)
    461461        alias = joins[-1]
    462462        col = target.column
    463463        if not field.rel:
    … …  
    511511            if not self.query.alias_refcount[alias]:
    512512                continue
    513513            try:
    514                 name, alias, join_type, lhs, lhs_col, col, nullable = self.query.alias_map[alias]
     514                name, alias, join_type, lhs, lhs_col, col, nullable, demoted_join = self.query.alias_map[alias]
    515515            except KeyError:
    516516                # Extra tables can end up in self.tables, but not in the
    517517                # alias_map if they aren't in a join. That's OK. We skip them.

  • django/db/models/sql/constants.py 

    diff -urN Django-1.4b1.orig/django/db/models/sql/constants.py Django-1.4b1/django/db/models/sql/constants.py
    old new  
    2424LHS_JOIN_COL = 4
    2525RHS_JOIN_COL = 5
    2626NULLABLE = 6
     27DEMOTED_JOIN = 7
    2728
    2829# How many results to expect from a cursor.execute call
    2930MULTI = 'multi'

  • django/db/models/sql/expressions.py 

    diff -urN Django-1.4b1.orig/django/db/models/sql/expressions.py Django-1.4b1/django/db/models/sql/expressions.py
    old new  
    4444            self.cols[node] = query.aggregate_select[node.name]
    4545        else:
    4646            try:
    47                 field, source, opts, join_list, last, _ = query.setup_joins(
     47                field, source, opts, join_list, last, _, _ = query.setup_joins(
    4848                    field_list, query.get_meta(),
    4949                    query.get_initial_alias(), False)
    5050                col, _, join_list = query.trim_joins(source, join_list, last, False)

  • django/db/models/sql/query.py 

    diff -urN Django-1.4b1.orig/django/db/models/sql/query.py Django-1.4b1/django/db/models/sql/query.py
    old new  
    504504                # the trimming of unnecessary tables.
    505505                if self.alias_refcount.get(alias) or rhs.alias_refcount.get(alias):
    506506                    self.promote_alias(alias, True)
     507            # Find aliases that are contained both in rhs and lhs
     508            # if one of them is demoted then promote lhs alias to LOUTER..
     509            common_tables = (l_tables & r_tables)
     510            for alias in common_tables:
     511                if alias in self.alias_map and alias in rhs.alias_map and \
     512                  self.alias_map[alias][DEMOTED_JOIN] != rhs.alias_map[alias][DEMOTED_JOIN]:
     513                    self.promote_alias(alias, True)
    507514
    508515        # Now relabel a copy of the rhs where-clause and add it to the current
    509516        # one.
    … …  
    696703                self.alias_map[alias][JOIN_TYPE] != self.LOUTER):
    697704            data = list(self.alias_map[alias])
    698705            data[JOIN_TYPE] = self.LOUTER
     706            data[DEMOTED_JOIN] = False
    699707            self.alias_map[alias] = tuple(data)
    700708            return True
    701709        return False
    702710
     711    def demote_alias(self, alias, change_join_type=False):
     712        """
     713        Demotes the join type of an alias to an inner join.
     714        """
     715        data = list(self.alias_map[alias])
     716        if change_join_type:
     717            data[JOIN_TYPE] = self.INNER
     718        data[DEMOTED_JOIN] = True
     719        self.alias_map[alias] = tuple(data)
     720
    703721    def promote_alias_chain(self, chain, must_promote=False):
    704722        """
    705723        Walks along a chain of aliases, promoting the first nullable join and
    … …  
    912930
    913931        # No reuse is possible, so we need a new alias.
    914932        alias, _ = self.table_alias(table, True)
     933
    915934        if not lhs:
    916935            # Not all tables need to be joined to anything. No join type
    917936            # means the later columns are ignored.
    918937            join_type = None
    919938        elif promote or outer_if_first:
    920939            join_type = self.LOUTER
     940        elif lhs in self.alias_map and self.alias_map[lhs][DEMOTED_JOIN]:
     941            # If the lhs alias was demoted to INNER
     942            # promote it to LOUTER and change join_type for rhs to LOUTER
     943            self.promote_alias(lhs, True)
     944            join_type = self.LOUTER
    921945        else:
    922946            join_type = self.INNER
    923         join = (table, alias, join_type, lhs, lhs_col, col, nullable)
     947        join = (table, alias, join_type, lhs, lhs_col, col, nullable, False)
    924948        self.alias_map[alias] = join
    925949        if t_ident in self.join_map:
    926950            self.join_map[t_ident] += (alias,)
    … …  
    10071031            #   - this is an annotation over a model field
    10081032            # then we need to explore the joins that are required.
    10091033
    1010             field, source, opts, join_list, last, _ = self.setup_joins(
     1034            field, source, opts, join_list, last, _, allow_trim_join = self.setup_joins(
    10111035                field_list, opts, self.get_initial_alias(), False)
    10121036
    10131037            # Process the join chain to see if it can be trimmed
    … …  
    11191143        allow_many = trim or not negate
    11201144
    11211145        try:
    1122             field, target, opts, join_list, last, extra_filters = self.setup_joins(
     1146            field, target, opts, join_list, last, extra_filters, allow_trim_join = self.setup_joins(
    11231147                    parts, opts, alias, True, allow_many, allow_explicit_fk=True,
    11241148                    can_reuse=can_reuse, negate=negate,
    11251149                    process_extras=process_extras)
    … …  
    11391163            self.promote_alias_chain(join_list)
    11401164            join_promote = True
    11411165
     1166        # If we have a one2one or many2one field, we can trim the left outer
     1167        # join from the end of a list of joins.
     1168        # In order to do this, we convert alias join type back to INNER, if possible,
     1169        # and trim_joins called later will do the strip for us.
     1170        # We have to be carefull when changing the join type to INNER, because
     1171        # the alias_map may already contain LOUTER join for this alias setup
     1172        # by some earlier applied filter.
     1173        # The join type can be changed to INNER by the demote_alias method only if
     1174        # it was set to LOUTER by a call to the promote_alias_chain above.
     1175        nonnull_comparison = lookup_type == 'isnull'
     1176        if nonnull_comparison and allow_trim_join and field.rel:
     1177                self.demote_alias(join_list[-1], change_join_type=join_promote)
     1178                nonnull_comparison = False
     1179
    11421180        # Process the join list to see if we can remove any inner joins from
    11431181        # the far end (fewer tables in a query is better).
    1144         nonnull_comparison = (lookup_type == 'isnull' and value is False)
    11451182        col, alias, join_list = self.trim_joins(target, join_list, last, trim,
    11461183                nonnull_comparison)
    11471184
    … …  
    12951332        dupe_set = set()
    12961333        exclusions = set()
    12971334        extra_filters = []
     1335        allow_trim_join = True
    12981336        int_alias = None
    12991337        for pos, name in enumerate(names):
     1338            allow_trim_join = True
    13001339            if int_alias is not None:
    13011340                exclusions.add(int_alias)
    13021341            exclusions.add(alias)
    … …  
    13181357                    raise FieldError("Cannot resolve keyword %r into field. "
    13191358                            "Choices are: %s" % (name, ", ".join(names)))
    13201359
     1360            # presence of indirect field in the filter requires
     1361            # left outer join for isnull
     1362            if not direct and allow_trim_join:
     1363                allow_trim_join = False
     1364
    13211365            if not allow_many and (m2m or not direct):
    13221366                for alias in joins:
    13231367                    self.unref_alias(alias)
    … …  
    14791523            else:
    14801524                raise FieldError("Join on field %r not permitted." % name)
    14811525
    1482         return field, target, opts, joins, last, extra_filters
     1526        return field, target, opts, joins, last, extra_filters, allow_trim_join
    14831527
    14841528    def trim_joins(self, target, join_list, last, trim, nonnull_check=False):
    14851529        """
    … …  
    16481692
    16491693        try:
    16501694            for name in field_names:
    1651                 field, target, u2, joins, u3, u4 = self.setup_joins(
     1695                field, target, u2, joins, u3, u4, allow_trim_join = self.setup_joins(
    16521696                        name.split(LOOKUP_SEP), opts, alias, False, allow_m2m,
    16531697                        True)
    16541698                final_alias = joins[-1]
    … …  
    19301974        """
    19311975        opts = self.model._meta
    19321976        alias = self.get_initial_alias()
    1933         field, col, opts, joins, last, extra = self.setup_joins(
     1977        field, col, opts, joins, last, extra, allow_trim_join = self.setup_joins(
    19341978                start.split(LOOKUP_SEP), opts, alias, False)
    19351979        select_col = self.alias_map[joins[1]][LHS_JOIN_COL]
    19361980        select_alias = alias

  • tests/regressiontests/queries/tests.py 

    diff -urN Django-1.4b1.orig/tests/regressiontests/queries/tests.py Django-1.4b1/tests/regressiontests/queries/tests.py
    old new  
    842842        )
    843843        Tag._meta.ordering = original_ordering
    844844
     845    def test_ticket10790(self):
     846
     847        """
     848            Querying direct fields with isnull should trim the left outer join.
     849           It also should not create INNER JOIN.
     850        """
     851        q = Tag.objects.filter(parent__isnull=True)
     852
     853        self.assertQuerysetEqual(q, ['<Tag: t1>'])
     854        self.assertTrue('JOIN' not in str(q.query))
     855
     856        q = Tag.objects.filter(parent__isnull=False)
     857
     858        self.assertQuerysetEqual(
     859            q,
     860            ['<Tag: t2>', '<Tag: t3>', '<Tag: t4>', '<Tag: t5>'],
     861        )
     862        self.assertTrue('JOIN' not in str(q.query))
     863
     864        q = Tag.objects.exclude(parent__isnull=True)
     865        self.assertQuerysetEqual(
     866            q,
     867            ['<Tag: t2>', '<Tag: t3>', '<Tag: t4>', '<Tag: t5>'],
     868        )
     869        self.assertTrue('JOIN' not in str(q.query))
     870
     871        q = Tag.objects.exclude(parent__isnull=False)
     872        self.assertQuerysetEqual( q, ['<Tag: t1>'])
     873        self.assertTrue('JOIN' not in str(q.query))
     874
     875        q = Tag.objects.exclude(parent__parent__isnull=False)
     876
     877        self.assertQuerysetEqual(
     878            q,
     879            ['<Tag: t1>', '<Tag: t2>', '<Tag: t3>'],
     880        )
     881        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 1)
     882        self.assertTrue('INNER JOIN' not in str(q.query))
     883
     884        """
     885            Querying across several tables should strip only the last outer join,
     886           while preserving the preceeding inner joins.
     887        """
     888        q = Tag.objects.filter(parent__parent__isnull=False)
     889
     890        self.assertQuerysetEqual(
     891            q,
     892            ['<Tag: t4>', '<Tag: t5>'],
     893        )
     894        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 0)
     895        self.assertTrue(str(q.query).count('INNER JOIN') == 1)
     896
     897        """
     898            Querying without isnull should not convert anything to left outer join.
     899        """
     900        # querying across the tables without isnull should strip the inner join
     901        q = Tag.objects.filter(parent__parent = self.t1)
     902        self.assertQuerysetEqual(
     903            q,
     904            ['<Tag: t4>', '<Tag: t5>'],
     905        )
     906        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 0)
     907        self.assertTrue(str(q.query).count('INNER JOIN') == 1)
     908
     909        """
     910            Querying via indirect fields should populate the left outer join
     911        """
     912        q = NamedCategory.objects.filter(tag__isnull=True)
     913        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 1)
     914        # join to dumbcategory ptr_id
     915        self.assertTrue(str(q.query).count('INNER JOIN') == 1)
     916        self.assertQuerysetEqual( q, [])
     917
     918        """
     919            Querying across several tables should strip only the last join, while
     920           preserving the preceding left outer joins.
     921        """
     922        q = NamedCategory.objects.filter(tag__parent__isnull=True)
     923        self.assertTrue(str(q.query).count('INNER JOIN') == 1)
     924        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 1)
     925        self.assertQuerysetEqual( q, ['<NamedCategory: NamedCategory object>'])
     926
     927        """
     928            Querying across m2m field should not strip the m2m table from join.
     929        """
     930        q = Author.objects.filter(item__tags__isnull=True)
     931        self.assertQuerysetEqual(
     932            q,
     933            ['<Author: a2>', '<Author: a3>'],
     934        )
     935        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 2)
     936        self.assertTrue('INNER JOIN' not in str(q.query))
     937
     938        q = Author.objects.filter(item__tags__parent__isnull=True)
     939        self.assertQuerysetEqual(
     940            q,
     941            ['<Author: a1>', '<Author: a2>', '<Author: a2>', '<Author: a3>'],
     942        )
     943        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 3)
     944        self.assertTrue('INNER JOIN' not in str(q.query))
     945
     946        """
     947            Querying with isnull=False across m2m field should not create outer joins
     948        """
     949        q = Author.objects.filter(item__tags__isnull=False)
     950        self.assertQuerysetEqual(
     951            q,
     952            ['<Author: a1>', '<Author: a1>', '<Author: a2>', '<Author: a2>', '<Author: a4>']
     953        )
     954        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 0)
     955        self.assertTrue(str(q.query).count('INNER JOIN') == 2)
     956
     957        q = Author.objects.filter(item__tags__parent__isnull=False)
     958        self.assertQuerysetEqual(
     959            q,
     960            ['<Author: a1>', '<Author: a2>', '<Author: a4>']
     961        )
     962        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 0)
     963        self.assertTrue(str(q.query).count('INNER JOIN') == 3)
     964
     965        q = Author.objects.filter(item__tags__parent__parent__isnull=False)
     966        self.assertQuerysetEqual(
     967            q,
     968            ['<Author: a4>']
     969        )
     970        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 0)
     971        self.assertTrue(str(q.query).count('INNER JOIN') == 4)
     972
     973        """
     974            Querying with isnull=True across m2m field should not create inner joins
     975           and strip last outer join
     976        """
     977        q = Author.objects.filter(item__tags__parent__parent__isnull=True)
     978        self.assertQuerysetEqual(
     979            q,
     980            ['<Author: a1>', '<Author: a1>', '<Author: a2>', '<Author: a2>', '<Author: a2>', '<Author: a3>']
     981        )
     982        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 4)
     983        self.assertTrue(str(q.query).count('INNER JOIN') == 0)
     984
     985        q = Author.objects.filter(item__tags__parent__isnull=True)
     986        self.assertQuerysetEqual(
     987            q,
     988            ['<Author: a1>', '<Author: a2>', '<Author: a2>', '<Author: a3>']
     989        )
     990        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 3)
     991        self.assertTrue(str(q.query).count('INNER JOIN') == 0)
     992
     993        """
     994            Reverse querying with isnull should not strip the join
     995        """
     996        q = Author.objects.filter(item__isnull=True)
     997        self.assertQuerysetEqual(
     998            q,
     999            ['<Author: a3>']
     1000        )
     1001        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 1)
     1002        self.assertTrue(str(q.query).count('INNER JOIN') == 0)
     1003
     1004        q = Author.objects.filter(item__isnull=False)
     1005        self.assertQuerysetEqual(
     1006            q,
     1007            ['<Author: a1>', '<Author: a2>', '<Author: a2>', '<Author: a4>']
     1008        )
     1009        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 0)
     1010        self.assertTrue(str(q.query).count('INNER JOIN') == 1)
     1011
     1012        """
     1013            Querying with combined q-objects should also strip the left outer join
     1014        """
     1015        q = Tag.objects.filter(Q(parent__isnull=True)|Q(parent=self.t1))
     1016        self.assertQuerysetEqual(
     1017            q,
     1018            ['<Tag: t1>', '<Tag: t2>', '<Tag: t3>']
     1019        )
     1020        self.assertTrue(str(q.query).count('LEFT OUTER JOIN') == 0)
     1021        self.assertTrue(str(q.query).count('INNER JOIN') == 0)
     1022
     1023        """
     1024            Combining queries should not re-populate the left outer join
     1025        """
     1026        q1 = Tag.objects.filter(parent__isnull=True)
     1027        q2 = Tag.objects.filter(parent__isnull=False)
     1028
     1029        q3 = q1|q2
     1030        self.assertQuerysetEqual(
     1031            q3,
     1032            ['<Tag: t1>', '<Tag: t2>', '<Tag: t3>', '<Tag: t4>', '<Tag: t5>'],
     1033        )
     1034        self.assertTrue(str(q3.query).count('LEFT OUTER JOIN') == 0)
     1035        self.assertTrue(str(q3.query).count('INNER JOIN') == 0)
     1036
     1037        q3 = q1&q2
     1038        self.assertQuerysetEqual( q3, [])
     1039        self.assertTrue(str(q3.query).count('LEFT OUTER JOIN') == 0)
     1040        self.assertTrue(str(q3.query).count('INNER JOIN') == 0)
     1041
     1042        q2 = Tag.objects.filter(parent = self.t1)
     1043        q3 = q1|q2
     1044        self.assertQuerysetEqual(
     1045            q3,
     1046            ['<Tag: t1>', '<Tag: t2>', '<Tag: t3>']
     1047        )
     1048        self.assertTrue(str(q3.query).count('LEFT OUTER JOIN') == 0)
     1049        self.assertTrue(str(q3.query).count('INNER JOIN') == 0)
     1050
     1051        q3 = q2|q1
     1052        self.assertQuerysetEqual(
     1053            q3,
     1054            ['<Tag: t1>', '<Tag: t2>', '<Tag: t3>']
     1055        )
     1056        self.assertTrue(str(q3.query).count('LEFT OUTER JOIN') == 0)
     1057        self.assertTrue(str(q3.query).count('INNER JOIN') == 0)
     1058
     1059        q1 = Tag.objects.filter(parent__isnull=True)
     1060        q2 = Tag.objects.filter(parent__parent__isnull=True)
     1061
     1062        q3 = q1|q2
     1063        self.assertQuerysetEqual(
     1064            q3,
     1065            ['<Tag: t1>', '<Tag: t2>', '<Tag: t3>']
     1066        )
     1067        self.assertTrue(str(q3.query).count('LEFT OUTER JOIN') == 1)
     1068        self.assertTrue(str(q3.query).count('INNER JOIN') == 0)
     1069
     1070        q3 = q2|q1
     1071        self.assertQuerysetEqual(
     1072            q3,
     1073            ['<Tag: t1>', '<Tag: t2>', '<Tag: t3>']
     1074        )
     1075        self.assertTrue(str(q3.query).count('LEFT OUTER JOIN') == 1)
     1076        self.assertTrue(str(q3.query).count('INNER JOIN') == 0)
     1077
    8451078class Queries2Tests(TestCase):
    8461079    def setUp(self):
    8471080        Number.objects.create(num=4)
Back to Top