Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

[1.2.X] 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. Backport of [14715].

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.2.X@14716 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 56dc22d64c08916508862784a3c97f27e1e6900b 1 parent c8e9d35
Alex Gaynor authored November 26, 2010
7  django/db/models/query.py
@@ -976,8 +976,7 @@ def iterator(self):
976 976
             # If a field list has been specified, use it. Otherwise, use the
977 977
             # full list of fields, including extras and aggregates.
978 978
             if self._fields:
979  
-                fields = list(self._fields) + filter(lambda f: f not in self._fields,
980  
-                                                     aggregate_names)
  979
+                fields = list(self._fields) + filter(lambda f: f not in self._fields, aggregate_names)
981 980
             else:
982 981
                 fields = names
983 982
 
@@ -987,7 +986,9 @@ def iterator(self):
987 986
 
988 987
     def _clone(self, *args, **kwargs):
989 988
         clone = super(ValuesListQuerySet, self)._clone(*args, **kwargs)
990  
-        clone.flat = self.flat
  989
+        if not hasattr(clone, "flat"):
  990
+            # Only assign flat if the clone didn't already get it from kwargs
  991
+            clone.flat = self.flat
991 992
         return clone
992 993
 
993 994
 
14  django/db/models/sql/compiler.py
@@ -466,9 +466,11 @@ def get_grouping(self):
466 466
         qn = self.quote_name_unless_alias
467 467
         result, params = [], []
468 468
         if self.query.group_by is not None:
469  
-            if len(self.query.model._meta.fields) == len(self.query.select) and \
470  
-                self.connection.features.allows_group_by_pk:
471  
-                self.query.group_by = [(self.query.model._meta.db_table, self.query.model._meta.pk.column)]
  469
+            if (len(self.query.model._meta.fields) == len(self.query.select) and
  470
+                self.connection.features.allows_group_by_pk):
  471
+                self.query.group_by = [
  472
+                    (self.query.model._meta.db_table, self.query.model._meta.pk.column)
  473
+                ]
472 474
 
473 475
             group_by = self.query.group_by or []
474 476
 
@@ -476,11 +478,13 @@ def get_grouping(self):
476 478
             for extra_select, extra_params in self.query.extra_select.itervalues():
477 479
                 extra_selects.append(extra_select)
478 480
                 params.extend(extra_params)
479  
-            for col in group_by + self.query.related_select_cols + extra_selects:
  481
+            cols = (group_by + self.query.select +
  482
+                self.query.related_select_cols + extra_selects)
  483
+            for col in cols:
480 484
                 if isinstance(col, (list, tuple)):
481 485
                     result.append('%s.%s' % (qn(col[0]), qn(col[1])))
482 486
                 elif hasattr(col, 'as_sql'):
483  
-                    result.append(col.as_sql(qn))
  487
+                    result.append(col.as_sql(qn, self.connection))
484 488
                 else:
485 489
                     result.append('(%s)' % str(col))
486 490
         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
@@ -194,7 +194,7 @@ def add_date_select(self, field, lookup_type, order='ASC'):
194 194
         alias = result[3][-1]
195 195
         select = Date((alias, field.column), lookup_type)
196 196
         self.select = [select]
197  
-        self.select_fields = [None]
  197
+        self.select_fields = []
198 198
         self.select_related = False # See #7097.
199 199
         self.set_extra_mask([])
200 200
         self.distinct = True
19  tests/regressiontests/aggregation_regress/tests.py
@@ -675,6 +675,25 @@ def test_f_expression_annotation(self):
675 675
             attrgetter("name")
676 676
         )
677 677
 
  678
+    def test_values_annotate_values(self):
  679
+        qs = Book.objects.values("name").annotate(
  680
+            n_authors=Count("authors")
  681
+        ).values_list("pk", flat=True)
  682
+        self.assertEqual(list(qs), list(Book.objects.values_list("pk", flat=True)))
  683
+
  684
+    def test_having_group_by(self):
  685
+        # Test that when a field occurs on the LHS of a HAVING clause that it
  686
+        # appears correctly in the GROUP BY clause
  687
+        qs = Book.objects.values_list("name").annotate(
  688
+            n_authors=Count("authors")
  689
+        ).filter(
  690
+            pages__gt=F("n_authors")
  691
+        ).values_list("name", flat=True)
  692
+        # Results should be the same, all Books have more pages than authors
  693
+        self.assertEqual(
  694
+            list(qs), list(Book.objects.values_list("name", flat=True))
  695
+        )
  696
+
678 697
     if run_stddev_tests():
679 698
         def test_stddev(self):
680 699
             self.assertEqual(
31  tests/regressiontests/queries/tests.py
@@ -1088,11 +1088,12 @@ def test_ticket7045(self):
1088 1088
         # Extra tables used to crash SQL construction on the second use.
1089 1089
         qs = Ranking.objects.extra(tables=['django_site'])
1090 1090
         qs.query.get_compiler(qs.db).as_sql()
1091  
-        qs.query.get_compiler(qs.db).as_sql()   # test passes if this doesn't raise an exception.
  1091
+        # test passes if this doesn't raise an exception.
  1092
+        qs.query.get_compiler(qs.db).as_sql()
1092 1093
 
1093 1094
     def test_ticket9848(self):
1094  
-        # Make sure that updates which only filter on sub-tables don't inadvertently
1095  
-        # update the wrong records (bug #9848).
  1095
+        # Make sure that updates which only filter on sub-tables don't
  1096
+        # inadvertently update the wrong records (bug #9848).
1096 1097
 
1097 1098
         # Make sure that the IDs from different tables don't happen to match.
1098 1099
         self.assertQuerysetEqual(
@@ -1278,15 +1279,15 @@ def test_tickets_8921_9188(self):
1278 1279
         )
1279 1280
 
1280 1281
         # The annotation->tag link is single values and tag->children links is
1281  
-        # multi-valued. So we have to split the exclude filter in the middle and then
1282  
-        # optimise the inner query without losing results.
  1282
+        # multi-valued. So we have to split the exclude filter in the middle
  1283
+        # and then optimise the inner query without losing results.
1283 1284
         self.assertQuerysetEqual(
1284 1285
             Annotation.objects.exclude(tag__children__name="t2"),
1285 1286
             ['<Annotation: a2>']
1286 1287
         )
1287 1288
 
1288  
-        # Nested queries are possible (although should be used with care, since they have
1289  
-        # performance problems on backends like MySQL.
  1289
+        # Nested queries are possible (although should be used with care, since
  1290
+        # they have performance problems on backends like MySQL.
1290 1291
 
1291 1292
         self.assertQuerysetEqual(
1292 1293
             Annotation.objects.filter(notes__in=Note.objects.filter(note="n1")),
@@ -1296,7 +1297,7 @@ def test_tickets_8921_9188(self):
1296 1297
     def test_ticket3739(self):
1297 1298
         # The all() method on querysets returns a copy of the queryset.
1298 1299
         q1 = Tag.objects.order_by('name')
1299  
-        self.assertNotEqual(id(q1), id(q1.all()))
  1300
+        self.assertTrue(q1 is not q1.all())
1300 1301
 
1301 1302
 
1302 1303
 class GeneratorExpressionTests(TestCase):
@@ -1447,6 +1448,16 @@ def test_values_subquery(self):
1447 1448
         )
1448 1449
 
1449 1450
 
  1451
+class ValuesQuerysetTests(BaseQuerysetTest):
  1452
+    def test_flat_values_lits(self):
  1453
+        Number.objects.create(num=72)
  1454
+        qs = Number.objects.values_list("num")
  1455
+        qs = qs.values_list("num", flat=True)
  1456
+        self.assertValueQuerysetEqual(
  1457
+            qs, [72]
  1458
+        )
  1459
+
  1460
+
1450 1461
 class WeirdQuerysetSlicingTests(BaseQuerysetTest):
1451 1462
     def setUp(self):
1452 1463
         Number.objects.create(num=1)
@@ -1472,8 +1483,8 @@ def test_tickets_7698_10202(self):
1472 1483
 class EscapingTests(TestCase):
1473 1484
     def test_ticket_7302(self):
1474 1485
         # Reserved names are appropriately escaped
1475  
-        _ = ReservedName.objects.create(name='a',order=42)
1476  
-        ReservedName.objects.create(name='b',order=37)
  1486
+        _ = ReservedName.objects.create(name='a', order=42)
  1487
+        ReservedName.objects.create(name='b', order=37)
1477 1488
         self.assertQuerysetEqual(
1478 1489
             ReservedName.objects.all().order_by('order'),
1479 1490
             ['<ReservedName: b>', '<ReservedName: a>']

0 notes on commit 56dc22d

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