Merge pull request #6 from django-nonrel/feature/refactor-ordering

Refactored _get_ordering

Also fixes some issues with ordering and custom `db_column`s.

Thanks to everybody involved!

djangotoolbox/db/basecompiler.py

@@ -188,16 +188,10 @@ def _matches_filters(self, entity, filters):
         return result
 
     def _order_in_memory(self, lhs, rhs):
-        for order in self.compiler._get_ordering():
-            if LOOKUP_SEP in order:
-                raise DatabaseError("JOINs in ordering not supported (%s)" % order)
-            if order == '?':
-                result = random.choice([1, 0, -1])
-            else:
-                column = order.lstrip('-')
-                result = cmp(lhs.get(column), rhs.get(column))
-                if order.startswith('-'):
-                    result *= -1
+        for column, descending in self.compiler._get_ordering():
+            result = cmp(lhs.get(column), rhs.get(column))
+            if descending:
+                result *= -1
             if result != 0:
                 return result
         return 0
@@ -329,33 +323,22 @@ def get_fields(self):
         return fields
 
     def _get_ordering(self):
+        opts = self.query.get_meta()
         if not self.query.default_ordering:
             ordering = self.query.order_by
         else:
-            ordering = self.query.order_by or self.query.get_meta().ordering
-        result = []
+            ordering = self.query.order_by or opts.ordering
         for order in ordering:
             if LOOKUP_SEP in order:
                 raise DatabaseError("Ordering can't span tables on non-relational backends (%s)" % order)
             if order == '?':
                 raise DatabaseError("Randomized ordering isn't supported by the backend")
-
             order = order.lstrip('+')
-
             descending = order.startswith('-')
-            name = order.lstrip('-')
-            if name == 'pk':
-                name = self.query.get_meta().pk.name
-                order = '-' + name if descending else name
-
-            if self.query.standard_ordering:
-                result.append(order)
-            else:
-                if descending:
-                    result.append(name)
-                else:
-                    result.append('-' + name)
-        return result
+            field = order.lstrip('-')
+            if field == 'pk':
+                field = opts.pk.name
+            yield (opts.get_field(field).column, descending)
 
 class NonrelInsertCompiler(object):
     def execute_sql(self, return_id=False):

djangotoolbox/tests.py

@@ -394,3 +394,20 @@ def test_select_related(self):
         source = Source.objects.all().select_related('target')[0]
         self.assertEqual(source.target.pk, target.pk)
         self.assertEqual(source.target.index, target.index)
+
+class OrderByTest(TestCase):
+    def test_foreign_keys(self):
+        target1 = Target.objects.create(index=1)
+        target2 = Target.objects.create(index=2)
+        source1 = Source.objects.create(target=target1, index=3)
+        source2 = Source.objects.create(target=target2, index=4)
+        self.assertEqual(list(Source.objects.all().order_by('target')), [source1, source2])
+        self.assertEqual(list(Source.objects.all().order_by('-target')), [source2, source1])
+
+    def test_db_column(self):
+        class DBColumn(models.Model):
+            a = models.IntegerField(db_column='b')
+        model1 = DBColumn.objects.create(a=1)
+        model2 = DBColumn.objects.create(a=2)
+        self.assertEqual(list(DBColumn.objects.all().order_by('a')), [model1, model2])
+        self.assertEqual(list(DBColumn.objects.all().order_by('-a')), [model2, model1])