Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #11916 -- Corrected handling of aggregation when there is a sub…

…query provided in an extra(select=) clause. Thanks to jaklaassen@gmail.com for the report, and to tobias, paluh, Karen Tracey and Ian Kelly for their work on the fix.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12896 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 4e5c20b78b17df25948809ca1c880551351d4d83 1 parent a75dc34
Russell Keith-Magee authored March 31, 2010
2  django/db/models/sql/compiler.py
@@ -484,7 +484,7 @@ def get_grouping(self):
484 484
                 elif hasattr(col, 'as_sql'):
485 485
                     result.append(col.as_sql(qn))
486 486
                 else:
487  
-                    result.append(str(col))
  487
+                    result.append('(%s)' % str(col))
488 488
         return result, params
489 489
 
490 490
     def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
30  tests/regressiontests/aggregation_regress/tests.py
... ...
@@ -1,5 +1,7 @@
  1
+from django.conf import settings
1 2
 from django.test import TestCase
2  
-from django.db.models import Max
  3
+from django.db import DEFAULT_DB_ALIAS
  4
+from django.db.models import Count, Max
3 5
 
4 6
 from regressiontests.aggregation_regress.models import *
5 7
 
@@ -13,7 +15,7 @@ def test_aggregates_in_where_clause(self):
13 15
 
14 16
         Tests that the subselect works and returns results equivalent to a
15 17
         query with the IDs listed.
16  
-        
  18
+
17 19
         Before the corresponding fix for this bug, this test passed in 1.1 and
18 20
         failed in 1.2-beta (trunk).
19 21
         """
@@ -33,7 +35,7 @@ def test_aggregates_in_where_clause_pre_eval(self):
33 35
 
34 36
         Same as the above test, but evaluates the queryset for the subquery
35 37
         before it's used as a subquery.
36  
-        
  38
+
37 39
         Before the corresponding fix for this bug, this test failed in both
38 40
         1.1 and 1.2-beta (trunk).
39 41
         """
@@ -46,3 +48,25 @@ def test_aggregates_in_where_clause_pre_eval(self):
46 48
         qs1 = books.filter(id__in=qs)
47 49
         qs2 = books.filter(id__in=list(qs))
48 50
         self.assertEqual(list(qs1), list(qs2))
  51
+
  52
+    if settings.DATABASES[DEFAULT_DB_ALIAS]['ENGINE'] != 'django.db.backends.oracle':
  53
+        def test_annotate_with_extra(self):
  54
+            """
  55
+            Regression test for #11916: Extra params + aggregation creates
  56
+            incorrect SQL.
  57
+            """
  58
+            #oracle doesn't support subqueries in group by clause
  59
+            shortest_book_sql = """
  60
+            SELECT name
  61
+            FROM aggregation_regress_book b
  62
+            WHERE b.publisher_id = aggregation_regress_publisher.id
  63
+            ORDER BY b.pages
  64
+            LIMIT 1
  65
+            """
  66
+            # tests that this query does not raise a DatabaseError due to the full
  67
+            # subselect being (erroneously) added to the GROUP BY parameters
  68
+            qs = Publisher.objects.extra(select={
  69
+                'name_of_shortest_book': shortest_book_sql,
  70
+            }).annotate(total_books=Count('book'))
  71
+            # force execution of the query
  72
+            list(qs)

0 notes on commit 4e5c20b

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