Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed a suite of errors in the ORM -- a) fixed calling values_list().…

…values_list() and changing whether the results are flat, b) fixed an issue with fields on the left-hand side of what becomes the HAVING clause not being included in the GROUP BY clause, and c) fixed a bug with fields from values() calls not being included in the GROUP BY clause. This fixed the recent test failures under postgresql.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@14715 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 678f626c24f389b03d4bfe2c552c33e9bdcc9379 1 parent 3cbaf3c
Alex Gaynor authored November 26, 2010
7  django/db/models/query.py
@@ -965,8 +965,7 @@ def iterator(self):
965 965
             # If a field list has been specified, use it. Otherwise, use the
966 966
             # full list of fields, including extras and aggregates.
967 967
             if self._fields:
968  
-                fields = list(self._fields) + filter(lambda f: f not in self._fields,
969  
-                                                     aggregate_names)
  968
+                fields = list(self._fields) + filter(lambda f: f not in self._fields, aggregate_names)
970 969
             else:
971 970
                 fields = names
972 971
 
@@ -976,7 +975,9 @@ def iterator(self):
976 975
 
977 976
     def _clone(self, *args, **kwargs):
978 977
         clone = super(ValuesListQuerySet, self)._clone(*args, **kwargs)
979  
-        clone.flat = self.flat
  978
+        if not hasattr(clone, "flat"):
  979
+            # Only assign flat if the clone didn't already get it from kwargs
  980
+            clone.flat = self.flat
980 981
         return clone
981 982
 
982 983
 
14  django/db/models/sql/compiler.py
@@ -469,9 +469,11 @@ def get_grouping(self):
469 469
         qn = self.quote_name_unless_alias
470 470
         result, params = [], []
471 471
         if self.query.group_by is not None:
472  
-            if len(self.query.model._meta.fields) == len(self.query.select) and \
473  
-                self.connection.features.allows_group_by_pk:
474  
-                self.query.group_by = [(self.query.model._meta.db_table, self.query.model._meta.pk.column)]
  472
+            if (len(self.query.model._meta.fields) == len(self.query.select) and
  473
+                self.connection.features.allows_group_by_pk):
  474
+                self.query.group_by = [
  475
+                    (self.query.model._meta.db_table, self.query.model._meta.pk.column)
  476
+                ]
475 477
 
476 478
             group_by = self.query.group_by or []
477 479
 
@@ -479,11 +481,13 @@ def get_grouping(self):
479 481
             for extra_select, extra_params in self.query.extra_select.itervalues():
480 482
                 extra_selects.append(extra_select)
481 483
                 params.extend(extra_params)
482  
-            for col in group_by + self.query.related_select_cols + extra_selects:
  484
+            cols = (group_by + self.query.select +
  485
+                self.query.related_select_cols + extra_selects)
  486
+            for col in cols:
483 487
                 if isinstance(col, (list, tuple)):
484 488
                     result.append('%s.%s' % (qn(col[0]), qn(col[1])))
485 489
                 elif hasattr(col, 'as_sql'):
486  
-                    result.append(col.as_sql(qn))
  490
+                    result.append(col.as_sql(qn, self.connection))
487 491
                 else:
488 492
                     result.append('(%s)' % str(col))
489 493
         return result, params
14  django/db/models/sql/query.py
@@ -195,8 +195,9 @@ def __setstate__(self, obj_dict):
195 195
         Unpickling support.
196 196
         """
197 197
         # Rebuild list of field instances
  198
+        opts = obj_dict['model']._meta
198 199
         obj_dict['select_fields'] = [
199  
-            name is not None and obj_dict['model']._meta.get_field(name) or None
  200
+            name is not None and opts.get_field(name) or None
200 201
             for name in obj_dict['select_fields']
201 202
         ]
202 203
 
@@ -707,13 +708,20 @@ def change_aliases(self, change_map):
707 708
         # "group by", "where" and "having".
708 709
         self.where.relabel_aliases(change_map)
709 710
         self.having.relabel_aliases(change_map)
710  
-        for columns in (self.select, self.aggregates.values(), self.group_by or []):
  711
+        for columns in [self.select, self.group_by or []]:
711 712
             for pos, col in enumerate(columns):
712 713
                 if isinstance(col, (list, tuple)):
713 714
                     old_alias = col[0]
714 715
                     columns[pos] = (change_map.get(old_alias, old_alias), col[1])
715 716
                 else:
716 717
                     col.relabel_aliases(change_map)
  718
+        for mapping in [self.aggregates]:
  719
+            for key, col in mapping.items():
  720
+                if isinstance(col, (list, tuple)):
  721
+                    old_alias = col[0]
  722
+                    mapping[key] = (change_map.get(old_alias, old_alias), col[1])
  723
+                else:
  724
+                    col.relabel_aliases(change_map)
717 725
 
718 726
         # 2. Rename the alias in the internal table/alias datastructures.
719 727
         for old_alias, new_alias in change_map.iteritems():
@@ -1075,6 +1083,8 @@ def add_filter(self, filter_expr, connector=AND, negate=False, trim=False,
1075 1083
 
1076 1084
 
1077 1085
         if having_clause:
  1086
+            if (alias, col) not in self.group_by:
  1087
+                self.group_by.append((alias, col))
1078 1088
             self.having.add((Constraint(alias, col, field), lookup_type, value),
1079 1089
                 connector)
1080 1090
         else:
2  django/db/models/sql/subqueries.py
@@ -202,7 +202,7 @@ def add_date_select(self, field_name, lookup_type, order='ASC'):
202 202
         alias = result[3][-1]
203 203
         select = Date((alias, field.column), lookup_type)
204 204
         self.select = [select]
205  
-        self.select_fields = [None]
  205
+        self.select_fields = []
206 206
         self.select_related = False # See #7097.
207 207
         self.set_extra_mask([])
208 208
         self.distinct = True
19  tests/regressiontests/aggregation_regress/tests.py
@@ -654,6 +654,25 @@ def test_f_expression_annotation(self):
654 654
             attrgetter("name")
655 655
         )
656 656
 
  657
+    def test_values_annotate_values(self):
  658
+        qs = Book.objects.values("name").annotate(
  659
+            n_authors=Count("authors")
  660
+        ).values_list("pk", flat=True)
  661
+        self.assertEqual(list(qs), list(Book.objects.values_list("pk", flat=True)))
  662
+
  663
+    def test_having_group_by(self):
  664
+        # Test that when a field occurs on the LHS of a HAVING clause that it
  665
+        # appears correctly in the GROUP BY clause
  666
+        qs = Book.objects.values_list("name").annotate(
  667
+            n_authors=Count("authors")
  668
+        ).filter(
  669
+            pages__gt=F("n_authors")
  670
+        ).values_list("name", flat=True)
  671
+        # Results should be the same, all Books have more pages than authors
  672
+        self.assertEqual(
  673
+            list(qs), list(Book.objects.values_list("name", flat=True))
  674
+        )
  675
+
657 676
     @skipUnlessDBFeature('supports_stddev')
658 677
     def test_stddev(self):
659 678
         self.assertEqual(
31  tests/regressiontests/queries/tests.py
@@ -1093,11 +1093,12 @@ def test_ticket7045(self):
1093 1093
         # Extra tables used to crash SQL construction on the second use.
1094 1094
         qs = Ranking.objects.extra(tables=['django_site'])
1095 1095
         qs.query.get_compiler(qs.db).as_sql()
1096  
-        qs.query.get_compiler(qs.db).as_sql()   # test passes if this doesn't raise an exception.
  1096
+        # test passes if this doesn't raise an exception.
  1097
+        qs.query.get_compiler(qs.db).as_sql()
1097 1098
 
1098 1099
     def test_ticket9848(self):
1099  
-        # Make sure that updates which only filter on sub-tables don't inadvertently
1100  
-        # update the wrong records (bug #9848).
  1100
+        # Make sure that updates which only filter on sub-tables don't
  1101
+        # inadvertently update the wrong records (bug #9848).
1101 1102
 
1102 1103
         # Make sure that the IDs from different tables don't happen to match.
1103 1104
         self.assertQuerysetEqual(
@@ -1283,15 +1284,15 @@ def test_tickets_8921_9188(self):
1283 1284
         )
1284 1285
 
1285 1286
         # The annotation->tag link is single values and tag->children links is
1286  
-        # multi-valued. So we have to split the exclude filter in the middle and then
1287  
-        # optimise the inner query without losing results.
  1287
+        # multi-valued. So we have to split the exclude filter in the middle
  1288
+        # and then optimise the inner query without losing results.
1288 1289
         self.assertQuerysetEqual(
1289 1290
             Annotation.objects.exclude(tag__children__name="t2"),
1290 1291
             ['<Annotation: a2>']
1291 1292
         )
1292 1293
 
1293  
-        # Nested queries are possible (although should be used with care, since they have
1294  
-        # performance problems on backends like MySQL.
  1294
+        # Nested queries are possible (although should be used with care, since
  1295
+        # they have performance problems on backends like MySQL.
1295 1296
 
1296 1297
         self.assertQuerysetEqual(
1297 1298
             Annotation.objects.filter(notes__in=Note.objects.filter(note="n1")),
@@ -1301,7 +1302,7 @@ def test_tickets_8921_9188(self):
1301 1302
     def test_ticket3739(self):
1302 1303
         # The all() method on querysets returns a copy of the queryset.
1303 1304
         q1 = Tag.objects.order_by('name')
1304  
-        self.assertNotEqual(id(q1), id(q1.all()))
  1305
+        self.assertIsNot(q1, q1.all())
1305 1306
 
1306 1307
 
1307 1308
 class GeneratorExpressionTests(TestCase):
@@ -1452,6 +1453,16 @@ def test_values_subquery(self):
1452 1453
         )
1453 1454
 
1454 1455
 
  1456
+class ValuesQuerysetTests(BaseQuerysetTest):
  1457
+    def test_flat_values_lits(self):
  1458
+        Number.objects.create(num=72)
  1459
+        qs = Number.objects.values_list("num")
  1460
+        qs = qs.values_list("num", flat=True)
  1461
+        self.assertValueQuerysetEqual(
  1462
+            qs, [72]
  1463
+        )
  1464
+
  1465
+
1455 1466
 class WeirdQuerysetSlicingTests(BaseQuerysetTest):
1456 1467
     def setUp(self):
1457 1468
         Number.objects.create(num=1)
@@ -1481,8 +1492,8 @@ def test_empty_resultset_sql(self):
1481 1492
 class EscapingTests(TestCase):
1482 1493
     def test_ticket_7302(self):
1483 1494
         # Reserved names are appropriately escaped
1484  
-        _ = ReservedName.objects.create(name='a',order=42)
1485  
-        ReservedName.objects.create(name='b',order=37)
  1495
+        _ = ReservedName.objects.create(name='a', order=42)
  1496
+        ReservedName.objects.create(name='b', order=37)
1486 1497
         self.assertQuerysetEqual(
1487 1498
             ReservedName.objects.all().order_by('order'),
1488 1499
             ['<ReservedName: b>', '<ReservedName: a>']

0 notes on commit 678f626

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