From bc87061a3c7c8d6b4d2469f35cc78683c6cff647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Tue, 19 May 2015 14:49:00 +0300 Subject: [PATCH] Fixed #24705 -- Fixed negated Q objects in expressions. Avoided split_exclude() for Q when used as an expression. --- django/db/models/query_utils.py | 2 +- django/db/models/sql/query.py | 12 ++++++---- docs/releases/1.8.2.txt | 4 ++++ tests/expressions_case/tests.py | 42 +++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-) diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py index b6caec386cb..8ac6e008ba0 100644 --- a/django/db/models/query_utils.py +++ b/django/db/models/query_utils.py @@ -90,7 +90,7 @@ class Q(tree.Node): # We must promote any new joins to left outer joins so that when Q is # used as an expression, rows aren't filtered due to joins. joins_before = query.tables[:] - clause, joins = query._add_q(self, reuse, allow_joins=allow_joins) + clause, joins = query._add_q(self, reuse, allow_joins=allow_joins, split_subq=False) joins_to_promote = [j for j in joins if j not in joins_before] query.promote_joins(joins_to_promote) return clause diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 776f06a7a81..e66695b591a 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1111,7 +1111,7 @@ class Query(object): (name, lhs.output_field.__class__.__name__)) def build_filter(self, filter_expr, branch_negated=False, current_negated=False, - can_reuse=None, connector=AND, allow_joins=True): + can_reuse=None, connector=AND, allow_joins=True, split_subq=True): """ Builds a WhereNode for a single filter clause, but doesn't add it to this Query. Query.add_q() will then add this filter to the where @@ -1158,7 +1158,7 @@ class Query(object): opts = self.get_meta() alias = self.get_initial_alias() - allow_many = not branch_negated + allow_many = not branch_negated or not split_subq try: field, sources, opts, join_list, path = self.setup_joins( @@ -1239,7 +1239,7 @@ class Query(object): self.demote_joins(existing_inner) def _add_q(self, q_object, used_aliases, branch_negated=False, - current_negated=False, allow_joins=True): + current_negated=False, allow_joins=True, split_subq=True): """ Adds a Q-object to the current filter. """ @@ -1253,12 +1253,14 @@ class Query(object): if isinstance(child, Node): child_clause, needed_inner = self._add_q( child, used_aliases, branch_negated, - current_negated, allow_joins) + current_negated, allow_joins, split_subq) joinpromoter.add_votes(needed_inner) else: child_clause, needed_inner = self.build_filter( child, can_reuse=used_aliases, branch_negated=branch_negated, - current_negated=current_negated, connector=connector, allow_joins=allow_joins) + current_negated=current_negated, connector=connector, + allow_joins=allow_joins, split_subq=split_subq, + ) joinpromoter.add_votes(needed_inner) if child_clause: target_clause.add(child_clause, connector) diff --git a/docs/releases/1.8.2.txt b/docs/releases/1.8.2.txt index 18c1ba38c92..635bc96e92a 100644 --- a/docs/releases/1.8.2.txt +++ b/docs/releases/1.8.2.txt @@ -18,6 +18,10 @@ Bugfixes query with a ``Case`` expression could unexpectedly filter out results (:ticket:`24766`). +* Fixed negated ``Q`` objects in expressions. Cases like + ``Case(When(~Q(friends__age__lte=30)))`` tried to generate a subquery which + resulted in a crash (:ticket:`24705`). + * Fixed incorrect GROUP BY clause generation on MySQL when the query's model has a self-referential foreign key (:ticket:`24748`). diff --git a/tests/expressions_case/tests.py b/tests/expressions_case/tests.py index 3479932865e..6d35be8865f 100644 --- a/tests/expressions_case/tests.py +++ b/tests/expressions_case/tests.py @@ -1048,6 +1048,48 @@ class CaseExpressionTests(TestCase): lambda x: (x, x.foo) ) + def test_m2m_exclude(self): + CaseTestModel.objects.create(integer=10, integer2=1, string='1') + qs = CaseTestModel.objects.values_list('id', 'integer').annotate( + cnt=models.Sum( + Case(When(~Q(fk_rel__integer=1), then=1), default=2), + output_field=models.IntegerField() + ), + ).order_by('integer') + # The first o has 2 as its fk_rel__integer=1, thus it hits the + # default=2 case. The other ones have 2 as the result as they have 2 + # fk_rel objects, except for integer=4 and integer=10 (created above). + # The integer=4 case has one integer, thus the result is 1, and + # integer=10 doesn't have any and this too generates 1 (instead of 0) + # as ~Q() also matches nulls. + self.assertQuerysetEqual( + qs, + [(1, 2), (2, 2), (2, 2), (3, 2), (3, 2), (3, 2), (4, 1), (10, 1)], + lambda x: x[1:] + ) + + def test_m2m_reuse(self): + CaseTestModel.objects.create(integer=10, integer2=1, string='1') + # Need to use values before annotate so that Oracle will not group + # by fields it isn't capable of grouping by. + qs = CaseTestModel.objects.values_list('id', 'integer').annotate( + cnt=models.Sum( + Case(When(~Q(fk_rel__integer=1), then=1), default=2), + output_field=models.IntegerField() + ), + ).annotate( + cnt2=models.Sum( + Case(When(~Q(fk_rel__integer=1), then=1), default=2), + output_field=models.IntegerField() + ), + ).order_by('integer') + self.assertEqual(str(qs.query).count(' JOIN '), 1) + self.assertQuerysetEqual( + qs, + [(1, 2, 2), (2, 2, 2), (2, 2, 2), (3, 2, 2), (3, 2, 2), (3, 2, 2), (4, 1, 1), (10, 1, 1)], + lambda x: x[1:] + ) + class CaseDocumentationExamples(TestCase): @classmethod