Skip to content

Commit

Permalink
Fixed #24705 -- Fixed negated Q objects in expressions.
Browse files Browse the repository at this point in the history
Avoided split_exclude() for Q when used as an expression.
  • Loading branch information
akaariai authored and timgraham committed May 20, 2015
1 parent 8b106cf commit bc87061
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 6 deletions.
2 changes: 1 addition & 1 deletion django/db/models/query_utils.py
Expand Up @@ -90,7 +90,7 @@ def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize
# 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
Expand Down
12 changes: 7 additions & 5 deletions django/db/models/sql/query.py
Expand Up @@ -1111,7 +1111,7 @@ def try_transform(self, lhs, name, rest_of_lookups):
(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
Expand Down Expand Up @@ -1158,7 +1158,7 @@ def build_filter(self, filter_expr, branch_negated=False, current_negated=False,

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(
Expand Down Expand Up @@ -1239,7 +1239,7 @@ def add_q(self, q_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.
"""
Expand All @@ -1253,12 +1253,14 @@ def _add_q(self, q_object, used_aliases, branch_negated=False,
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)
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/1.8.2.txt
Expand Up @@ -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`).

Expand Down
42 changes: 42 additions & 0 deletions tests/expressions_case/tests.py
Expand Up @@ -1048,6 +1048,48 @@ def test_join_promotion(self):
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
Expand Down

0 comments on commit bc87061

Please sign in to comment.