Fixed #11916 -- Corrected handling of aggregation when there is a subquery 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

2 files changed, 28 insertions, 4 deletions

diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index 2fe03302a9..a07e506fc7 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -484,7 +484,7 @@ class SQLCompiler(object):
                 elif hasattr(col, 'as_sql'):
                     result.append(col.as_sql(qn))
                 else:
-                    result.append(str(col))
+                    result.append('(%s)' % str(col))
         return result, params
 
     def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
diff --git a/tests/regressiontests/aggregation_regress/tests.py b/tests/regressiontests/aggregation_regress/tests.py
index cf0f58a9a8..3c4bdfa47d 100644
--- a/tests/regressiontests/aggregation_regress/tests.py
+++ b/tests/regressiontests/aggregation_regress/tests.py
@@ -1,5 +1,7 @@
+from django.conf import settings
 from django.test import TestCase
-from django.db.models import Max
+from django.db import DEFAULT_DB_ALIAS
+from django.db.models import Count, Max
 
 from regressiontests.aggregation_regress.models import *
 
@@ -13,7 +15,7 @@ class AggregationTests(TestCase):
 
         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).
         """
@@ -33,7 +35,7 @@ class AggregationTests(TestCase):
 
         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).
         """
@@ -46,3 +48,25 @@ class AggregationTests(TestCase):
         qs1 = books.filter(id__in=qs)
         qs2 = books.filter(id__in=list(qs))
         self.assertEqual(list(qs1), list(qs2))
+
+    if settings.DATABASES[DEFAULT_DB_ALIAS]['ENGINE'] != 'django.db.backends.oracle':
+        def test_annotate_with_extra(self):
+            """
+            Regression test for #11916: Extra params + aggregation creates
+            incorrect SQL.
+            """
+            #oracle doesn't support subqueries in group by clause
+            shortest_book_sql = """
+            SELECT name
+            FROM aggregation_regress_book b
+            WHERE b.publisher_id = aggregation_regress_publisher.id
+            ORDER BY b.pages
+            LIMIT 1
+            """
+            # tests that this query does not raise a DatabaseError due to the full
+            # subselect being (erroneously) added to the GROUP BY parameters
+            qs = Publisher.objects.extra(select={
+                'name_of_shortest_book': shortest_book_sql,
+            }).annotate(total_books=Count('book'))
+            # force execution of the query
+            list(qs)