Ticket #14930: extra_order_by_values_list_with_tests_3.diff

django/db/models/query.py

@@ -1017 +1017 @@
 
         names = extra_names + field_names + aggregate_names
 
+        # If a field list has been specified, use it. Otherwise, use the
+        # full list of fields, including extras and aggregates.
+        if self._fields:
+            fields = list(self._fields) + [f for f in aggregate_names if f not in self._fields]
+        else:
+            fields = names
+
         for row in self.query.get_compiler(self.db).results_iter():
-            yield dict(zip(names, row))
+            # removes the non-necessary fields in the intersection
+            # between names and fields
+            yield {key: elem for key, elem in zip(names, row) if key in fields}
 
     def delete(self):
         # values().delete() doesn't work currently - make sure it raises an
@@ -1064 +1073 @@
             self.aggregate_names = None
 
         self.query.select = []
+        # add fields that are required by order_by or extra_order_by and
+        # present in extra_select to extra_mask
+        order_by_fields = list(self.query.order_by or []) + list(self.query.extra_order_by or [])
+        extra_mask_names = list(self.extra_names or []) +\
+            [n for n in order_by_fields if n in self.query.extra_select]
         if self.extra_names is not None:
-            self.query.set_extra_mask(self.extra_names)
+            self.query.set_extra_mask(extra_mask_names)
         self.query.add_fields(self.field_names, True)
         if self.aggregate_names is not None:
             self.query.set_aggregate_mask(self.aggregate_names)
@@ -1139 +1153 @@
 class ValuesListQuerySet(ValuesQuerySet):
     def iterator(self):
         if self.flat and len(self._fields) == 1:
+            # get fields that were added to extra_select_mask but are not
+            # expected in the value_list (e.g: a field was added to
+            # extra_select_mask because it was needed be an order by clause)
+            extra_fields = [f for f in self.query.extra_select_mask if f not in self._fields]
             for row in self.query.get_compiler(self.db).results_iter():
-                yield row[0]
+                yield row[len(extra_fields)]
         elif not self.query.extra_select and not self.query.aggregate_select:
             for row in self.query.get_compiler(self.db).results_iter():
                 yield tuple(row)

tests/regressiontests/queries/tests.py

@@ -1531 +1531 @@
         # An empty values() call includes all aliases, including those from an
         # extra()
         qs = Ranking.objects.extra(select={'good': 'case when rank > 2 then 1 else 0 end'})
-        dicts = qs.values().order_by('id')
+        dicts = qs.values()
+        dicts = dicts.order_by('id')
         for d in dicts: del d['id']; del d['author_id']
         self.assertEqual(
             [sorted(d.items()) for d in dicts],
@@ -1995 +1996 @@
 
 
 class ValuesQuerysetTests(BaseQuerysetTest):
-    def test_flat_values_lits(self):
+    def setUp(self):
         Number.objects.create(num=72)
+
+    def test_flat_values_list(self):
         qs = Number.objects.values_list("num")
         qs = qs.values_list("num", flat=True)
         self.assertValueQuerysetEqual(
             qs, [72]
         )
 
+    def test_extra_values(self):
+        # testing for ticket 14930 issues
+        qs = Number.objects.extra(select={'value_plus_one': 'num+1', 'value_minus_one': 'num-1' })
+        qs = qs.order_by('value_minus_one')
+        qs = qs.values('num')
+        identity = lambda x:x
+        self.assertQuerysetEqual(qs, [{'num': 72}], identity)
+
+    def test_extra_values_order_twice(self):
+        # testing for ticket 14930 issues
+        qs = Number.objects.extra(select={'value_plus_one': 'num+1', 'value_minus_one': 'num-1' })
+        qs = qs.order_by('value_minus_one').order_by('value_plus_one')
+        qs = qs.values('num')
+        identity = lambda x:x
+        self.assertQuerysetEqual(qs, [{'num': 72}], identity)
+
+    def test_extra_values_order_in_extra(self):
+        # testing for ticket 14930 issues
+        qs = Number.objects.extra(
+                select={'value_plus_one': 'num+1', 'value_minus_one': 'num-1' },
+                order_by=['value_minus_one'])
+        qs = qs.values('num')
+        identity = lambda x:x
+        self.assertQuerysetEqual(qs, [{'num': 72}], identity)
+
+    def test_extra_values_list(self):
+        # testing for ticket 14930 issues
+        qs = Number.objects.extra(select={'value_plus_one': 'num+1'})
+        qs = qs.order_by('value_plus_one')
+        qs = qs.values_list('num')
+        identity = lambda x:x
+        self.assertQuerysetEqual(qs, [(72,)], identity)
+
+    def test_flat_extra_values_list(self):
+        # testing for ticket 14930 issues
+        qs = Number.objects.extra(select={'value_plus_one': 'num+1'})
+        qs = qs.order_by('value_plus_one')
+        qs = qs.values_list('num', flat=True)
+        identity = lambda x:x
+        self.assertQuerysetEqual(qs, [72], identity)
+
+
 
 class WeirdQuerysetSlicingTests(BaseQuerysetTest):
     def setUp(self):