From fd3fa851b592700a8b04af46f626454db0db02e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Wed, 8 Jan 2014 19:35:47 +0200 Subject: [PATCH] [1.6.x] Fixed #21748 -- join promotion for negated AND conditions Made sure Django treats case .filter(NOT (a AND b)) the same way as .filter((NOT a OR NOT b)) for join promotion. Heavily modified backpatch of 35cecb1ebd0ccda0be7a518d1b7273333d26fbae from master. Conflicts: django/db/models/sql/query.py tests/queries/tests.py --- django/db/models/sql/query.py | 16 +++---- tests/queries/tests.py | 79 +++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index bb05c94e14c..7868c19ab8e 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1211,16 +1211,18 @@ class Query(object): connector = q_object.connector current_negated = current_negated ^ q_object.negated branch_negated = branch_negated or q_object.negated - # Note that if the connector happens to match what we have already in - # the tree, the add will be a no-op. target_clause = self.where_class(connector=connector, negated=q_object.negated) - - if connector == OR: + # Treat case NOT (a AND b) like case ((NOT a) OR (NOT b)) for join + # promotion. See ticket #21748. + effective_connector = connector + if current_negated: + effective_connector = OR if effective_connector == AND else AND + if effective_connector == OR: alias_usage_counts = dict() aliases_before = set(self.tables) for child in q_object.children: - if connector == OR: + if effective_connector == OR: refcounts_before = self.alias_refcount.copy() if isinstance(child, Node): child_clause = self._add_q( @@ -1231,11 +1233,11 @@ class Query(object): child, can_reuse=used_aliases, branch_negated=branch_negated, current_negated=current_negated) target_clause.add(child_clause, connector) - if connector == OR: + if effective_connector == OR: used = alias_diff(refcounts_before, self.alias_refcount) for alias in used: alias_usage_counts[alias] = alias_usage_counts.get(alias, 0) + 1 - if connector == OR: + if effective_connector == OR: self.promote_disjunction(aliases_before, alias_usage_counts, len(q_object.children)) return target_clause diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 6d59df6b0ed..e38d573e804 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2662,6 +2662,85 @@ class NullJoinPromotionOrTest(TestCase): self.assertEqual(str(qs.query).count('INNER JOIN'), 1) self.assertEqual(list(qs), [self.a2]) + def test_ticket_21748(self): + i1 = Identifier.objects.create(name='i1') + i2 = Identifier.objects.create(name='i2') + i3 = Identifier.objects.create(name='i3') + Program.objects.create(identifier=i1) + Channel.objects.create(identifier=i1) + Program.objects.create(identifier=i2) + self.assertQuerysetEqual( + Identifier.objects.filter(program=None, channel=None), + [i3], lambda x: x) + self.assertQuerysetEqual( + Identifier.objects.exclude(program=None, channel=None).order_by('name'), + [i1, i2], lambda x: x) + + def test_ticket_21748_double_negated_and(self): + i1 = Identifier.objects.create(name='i1') + i2 = Identifier.objects.create(name='i2') + Identifier.objects.create(name='i3') + p1 = Program.objects.create(identifier=i1) + c1 = Channel.objects.create(identifier=i1) + Program.objects.create(identifier=i2) + # Check the ~~Q() (or equivalently .exclude(~Q)) works like Q() for + # join promotion. + qs1_doubleneg = Identifier.objects.exclude(~Q(program__id=p1.id, channel__id=c1.id)).order_by('pk') + qs1_filter = Identifier.objects.filter(program__id=p1.id, channel__id=c1.id).order_by('pk') + self.assertQuerysetEqual(qs1_doubleneg, qs1_filter, lambda x: x) + self.assertEqual(str(qs1_filter.query).count('JOIN'), + str(qs1_doubleneg.query).count('JOIN')) + self.assertEqual(2, str(qs1_doubleneg.query).count('INNER JOIN')) + self.assertEqual(str(qs1_filter.query).count('INNER JOIN'), + str(qs1_doubleneg.query).count('INNER JOIN')) + + def test_ticket_21748_double_negated_or(self): + i1 = Identifier.objects.create(name='i1') + i2 = Identifier.objects.create(name='i2') + Identifier.objects.create(name='i3') + p1 = Program.objects.create(identifier=i1) + c1 = Channel.objects.create(identifier=i1) + p2 = Program.objects.create(identifier=i2) + # Test OR + doubleneq. The expected result is that channel is LOUTER + # joined, program INNER joined + qs1_filter = Identifier.objects.filter( + Q(program__id=p2.id, channel__id=c1.id) + | Q(program__id=p1.id) + ).order_by('pk') + qs1_doubleneg = Identifier.objects.exclude( + ~Q(Q(program__id=p2.id, channel__id=c1.id) + | Q(program__id=p1.id)) + ).order_by('pk') + self.assertQuerysetEqual(qs1_doubleneg, qs1_filter, lambda x: x) + self.assertEqual(str(qs1_filter.query).count('JOIN'), + str(qs1_doubleneg.query).count('JOIN')) + self.assertEqual(1, str(qs1_doubleneg.query).count('INNER JOIN')) + self.assertEqual(str(qs1_filter.query).count('INNER JOIN'), + str(qs1_doubleneg.query).count('INNER JOIN')) + + def test_ticket_21748_complex_filter(self): + i1 = Identifier.objects.create(name='i1') + i2 = Identifier.objects.create(name='i2') + Identifier.objects.create(name='i3') + p1 = Program.objects.create(identifier=i1) + c1 = Channel.objects.create(identifier=i1) + p2 = Program.objects.create(identifier=i2) + # Finally, a more complex case, one time in a way where each + # NOT is pushed to lowest level in the boolean tree, and + # another query where this isn't done. + qs1 = Identifier.objects.filter( + ~Q(~Q(program__id=p2.id, channel__id=c1.id) + & Q(program__id=p1.id))).order_by('pk') + qs2 = Identifier.objects.filter( + Q(Q(program__id=p2.id, channel__id=c1.id) + | ~Q(program__id=p1.id))).order_by('pk') + self.assertQuerysetEqual(qs1, qs2, lambda x: x) + self.assertEqual(str(qs1.query).count('JOIN'), + str(qs2.query).count('JOIN')) + self.assertEqual(0, str(qs1.query).count('INNER JOIN')) + self.assertEqual(str(qs1.query).count('INNER JOIN'), + str(qs2.query).count('INNER JOIN')) + class ReverseJoinTrimmingTest(TestCase): def test_reverse_trimming(self):