Skip to content

Commit

Permalink
[1.1.X] Fixed #12822: Don't copy the _aggregate_select_cache when clo…
Browse files Browse the repository at this point in the history
…ning a query set,

as that can lead to incorrect SQL being generated for the query. Thanks to mat
for the report and test, tobias for the fix, and Alex for review. 

r12830 from trunk.


git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.1.X@12831 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information
kmtracey committed Mar 21, 2010
1 parent 3a55a2f commit 740c974
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 4 deletions.
10 changes: 6 additions & 4 deletions django/db/models/sql/query.py
Expand Up @@ -215,10 +215,12 @@ def clone(self, klass=None, memo=None, **kwargs):
obj.aggregate_select_mask = None obj.aggregate_select_mask = None
else: else:
obj.aggregate_select_mask = self.aggregate_select_mask.copy() obj.aggregate_select_mask = self.aggregate_select_mask.copy()
if self._aggregate_select_cache is None: # _aggregate_select_cache cannot be copied, as doing so breaks the
obj._aggregate_select_cache = None # (necessary) state in which both aggregates and
else: # _aggregate_select_cache point to the same underlying objects.
obj._aggregate_select_cache = self._aggregate_select_cache.copy() # It will get re-populated in the cloned queryset the next time it's
# used.
obj._aggregate_select_cache = None
obj.max_depth = self.max_depth obj.max_depth = self.max_depth
obj.extra = self.extra.copy() obj.extra = self.extra.copy()
if self.extra_select_mask is None: if self.extra_select_mask is None:
Expand Down
48 changes: 48 additions & 0 deletions tests/regressiontests/aggregation_regress/tests.py
@@ -0,0 +1,48 @@
from django.test import TestCase
from django.db.models import Max

from regressiontests.aggregation_regress.models import *


class AggregationTests(TestCase):

def test_aggregates_in_where_clause(self):
"""
Regression test for #12822: DatabaseError: aggregates not allowed in
WHERE clause
Tests that the subselect works and returns results equivalent to a
query with the IDs listed.
Before the corresponding fix for this bug, this test passed in 1.1 and
failed in 1.2-beta (trunk).
"""
qs = Book.objects.values('contact').annotate(Max('id'))
qs = qs.order_by('contact').values_list('id__max', flat=True)
# don't do anything with the queryset (qs) before including it as a
# subquery
books = Book.objects.order_by('id')
qs1 = books.filter(id__in=qs)
qs2 = books.filter(id__in=list(qs))
self.assertEqual(list(qs1), list(qs2))

def test_aggregates_in_where_clause_pre_eval(self):
"""
Regression test for #12822: DatabaseError: aggregates not allowed in
WHERE clause
Same as the above test, but evaluates the queryset for the subquery
before it's used as a subquery.
Before the corresponding fix for this bug, this test failed in both
1.1 and 1.2-beta (trunk).
"""
qs = Book.objects.values('contact').annotate(Max('id'))
qs = qs.order_by('contact').values_list('id__max', flat=True)
# force the queryset (qs) for the subquery to be evaluated in its
# current state
list(qs)
books = Book.objects.order_by('id')
qs1 = books.filter(id__in=qs)
qs2 = books.filter(id__in=list(qs))
self.assertEqual(list(qs1), list(qs2))

0 comments on commit 740c974

Please sign in to comment.