Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #12822: Don't copy the _aggregate_select_cache when cloning a q…

…uery 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.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@12830 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 9df71371c2d70f61ae0f8916422487bb031a7164 1 parent 72ea8fc
Karen Tracey authored March 21, 2010
10  django/db/models/sql/query.py
@@ -240,10 +240,12 @@ def clone(self, klass=None, memo=None, **kwargs):
240 240
             obj.aggregate_select_mask = None
241 241
         else:
242 242
             obj.aggregate_select_mask = self.aggregate_select_mask.copy()
243  
-        if self._aggregate_select_cache is None:
244  
-            obj._aggregate_select_cache = None
245  
-        else:
246  
-            obj._aggregate_select_cache = self._aggregate_select_cache.copy()
  243
+        # _aggregate_select_cache cannot be copied, as doing so breaks the
  244
+        # (necessary) state in which both aggregates and
  245
+        # _aggregate_select_cache point to the same underlying objects.
  246
+        # It will get re-populated in the cloned queryset the next time it's
  247
+        # used.
  248
+        obj._aggregate_select_cache = None
247 249
         obj.max_depth = self.max_depth
248 250
         obj.extra = self.extra.copy()
249 251
         if self.extra_select_mask is None:
48  tests/regressiontests/aggregation_regress/tests.py
... ...
@@ -0,0 +1,48 @@
  1
+from django.test import TestCase
  2
+from django.db.models import Max
  3
+
  4
+from regressiontests.aggregation_regress.models import *
  5
+
  6
+
  7
+class AggregationTests(TestCase):
  8
+
  9
+    def test_aggregates_in_where_clause(self):
  10
+        """
  11
+        Regression test for #12822: DatabaseError: aggregates not allowed in
  12
+        WHERE clause
  13
+
  14
+        Tests that the subselect works and returns results equivalent to a
  15
+        query with the IDs listed.
  16
+        
  17
+        Before the corresponding fix for this bug, this test passed in 1.1 and
  18
+        failed in 1.2-beta (trunk).
  19
+        """
  20
+        qs = Book.objects.values('contact').annotate(Max('id'))
  21
+        qs = qs.order_by('contact').values_list('id__max', flat=True)
  22
+        # don't do anything with the queryset (qs) before including it as a
  23
+        # subquery
  24
+        books = Book.objects.order_by('id')
  25
+        qs1 = books.filter(id__in=qs)
  26
+        qs2 = books.filter(id__in=list(qs))
  27
+        self.assertEqual(list(qs1), list(qs2))
  28
+
  29
+    def test_aggregates_in_where_clause_pre_eval(self):
  30
+        """
  31
+        Regression test for #12822: DatabaseError: aggregates not allowed in
  32
+        WHERE clause
  33
+
  34
+        Same as the above test, but evaluates the queryset for the subquery
  35
+        before it's used as a subquery.
  36
+        
  37
+        Before the corresponding fix for this bug, this test failed in both
  38
+        1.1 and 1.2-beta (trunk).
  39
+        """
  40
+        qs = Book.objects.values('contact').annotate(Max('id'))
  41
+        qs = qs.order_by('contact').values_list('id__max', flat=True)
  42
+        # force the queryset (qs) for the subquery to be evaluated in its
  43
+        # current state
  44
+        list(qs)
  45
+        books = Book.objects.order_by('id')
  46
+        qs1 = books.filter(id__in=qs)
  47
+        qs2 = books.filter(id__in=list(qs))
  48
+        self.assertEqual(list(qs1), list(qs2))

0 notes on commit 9df7137

Please sign in to comment.
Something went wrong with that request. Please try again.