Fixed #24060 -- Added OrderBy Expressions

2 files changed, 58 insertions, 34 deletions

diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
index 7d9b3ee609..b091571835 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -1,9 +1,10 @@
 from itertools import chain
+import re
 import warnings
 
 from django.core.exceptions import FieldError
 from django.db.models.constants import LOOKUP_SEP
-from django.db.models.expressions import RawSQL, Ref, Random, ColIndexRef
+from django.db.models.expressions import OrderBy, Random, RawSQL, Ref
 from django.db.models.query_utils import select_related_descend, QueryWrapper
 from django.db.models.sql.constants import (CURSOR, SINGLE, MULTI, NO_RESULTS,
         ORDER_DIR, GET_ITERATOR_CHUNK_SIZE)
@@ -28,6 +29,7 @@ class SQLCompiler(object):
         self.select = None
         self.annotation_col_map = None
         self.klass_info = None
+        self.ordering_parts = re.compile(r'(.*)\s(ASC|DESC)(.*)')
 
     def setup_query(self):
         if all(self.query.alias_refcount[a] == 0 for a in self.query.tables):
@@ -105,14 +107,14 @@ class SQLCompiler(object):
             cols = expr.get_group_by_cols()
             for col in cols:
                 expressions.append(col)
-        for expr, _ in order_by:
+        for expr, (sql, params, is_ref) in order_by:
             if expr.contains_aggregate:
                 continue
             # We can skip References to select clause, as all expressions in
             # the select clause are already part of the group by.
-            if isinstance(expr, Ref):
+            if is_ref:
                 continue
-            expressions.append(expr)
+            expressions.extend(expr.get_source_expressions())
         having = self.query.having.get_group_by_cols()
         for expr in having:
             expressions.append(expr)
@@ -234,54 +236,75 @@ class SQLCompiler(object):
 
         order_by = []
         for pos, field in enumerate(ordering):
-            if field == '?':
-                order_by.append((Random(), asc, False))
+            if hasattr(field, 'resolve_expression'):
+                if not isinstance(field, OrderBy):
+                    field = field.asc()
+                if not self.query.standard_ordering:
+                    field.reverse_ordering()
+                order_by.append((field, False))
                 continue
-            if isinstance(field, int):
-                if field < 0:
-                    field = -field
-                    int_ord = desc
-                order_by.append((ColIndexRef(field), int_ord, True))
+            if field == '?':  # random
+                order_by.append((OrderBy(Random()), False))
                 continue
+
             col, order = get_order_dir(field, asc)
+            descending = True if order == 'DESC' else False
+
             if col in self.query.annotation_select:
-                order_by.append((Ref(col, self.query.annotation_select[col]), order, True))
+                order_by.append((
+                    OrderBy(Ref(col, self.query.annotation_select[col]), descending=descending),
+                    True))
                 continue
+
             if '.' in field:
                 # This came in through an extra(order_by=...) addition. Pass it
                 # on verbatim.
                 table, col = col.split('.', 1)
-                expr = RawSQL('%s.%s' % (self.quote_name_unless_alias(table), col), [])
-                order_by.append((expr, order, False))
+                order_by.append((
+                    OrderBy(RawSQL('%s.%s' % (self.quote_name_unless_alias(table), col), [])),
+                    False))
                 continue
-            if not self.query._extra or get_order_dir(field)[0] not in self.query._extra:
+
+            if not self.query._extra or col not in self.query._extra:
                 # 'col' is of the form 'field' or 'field1__field2' or
                 # '-field1__field2__field', etc.
-                order_by.extend(self.find_ordering_name(field, self.query.get_meta(),
-                                                        default_order=asc))
+                order_by.extend(self.find_ordering_name(
+                    field, self.query.get_meta(), default_order=asc))
             else:
                 if col not in self.query.extra_select:
-                    order_by.append((RawSQL(*self.query.extra[col]), order, False))
+                    order_by.append((
+                        OrderBy(RawSQL(*self.query.extra[col]), descending=descending),
+                        False))
                 else:
-                    order_by.append((Ref(col, RawSQL(*self.query.extra[col])),
-                                     order, True))
+                    order_by.append((
+                        OrderBy(Ref(col, RawSQL(*self.query.extra[col])), descending=descending),
+                        True))
         result = []
         seen = set()
-        for expr, order, is_ref in order_by:
-            sql, params = self.compile(expr)
-            if (sql, tuple(params)) in seen:
+
+        for expr, is_ref in order_by:
+            resolved = expr.resolve_expression(
+                self.query, allow_joins=True, reuse=None)
+            sql, params = self.compile(resolved)
+            # Don't add the same column twice, but the order direction is
+            # not taken into account so we strip it. When this entire method
+            # is refactored into expressions, then we can check each part as we
+            # generate it.
+            without_ordering = self.ordering_parts.search(sql).group(1)
+            if (without_ordering, tuple(params)) in seen:
                 continue
-            seen.add((sql, tuple(params)))
-            result.append((expr, (sql, params, order, is_ref)))
+            seen.add((without_ordering, tuple(params)))
+            result.append((resolved, (sql, params, is_ref)))
         return result
 
     def get_extra_select(self, order_by, select):
         extra_select = []
         select_sql = [t[1] for t in select]
         if self.query.distinct and not self.query.distinct_fields:
-            for expr, (sql, params, _, is_ref) in order_by:
-                if not is_ref and (sql, params) not in select_sql:
-                    extra_select.append((expr, (sql, params), None))
+            for expr, (sql, params, is_ref) in order_by:
+                without_ordering = self.ordering_parts.search(sql).group(1)
+                if not is_ref and (without_ordering, params) not in select_sql:
+                    extra_select.append((expr, (without_ordering, params), None))
         return extra_select
 
     def __call__(self, name):
@@ -392,8 +415,8 @@ class SQLCompiler(object):
 
             if order_by:
                 ordering = []
-                for _, (o_sql, o_params, order, _) in order_by:
-                    ordering.append('%s %s' % (o_sql, order))
+                for _, (o_sql, o_params, _) in order_by:
+                    ordering.append(o_sql)
                     params.extend(o_params)
                 result.append('ORDER BY %s' % ', '.join(ordering))
 
@@ -514,6 +537,7 @@ class SQLCompiler(object):
         The 'name' is of the form 'field1__field2__...__fieldN'.
         """
         name, order = get_order_dir(name, default_order)
+        descending = True if order == 'DESC' else False
         pieces = name.split(LOOKUP_SEP)
         field, targets, alias, joins, path, opts = self._setup_joins(pieces, opts, alias)
 
@@ -535,7 +559,7 @@ class SQLCompiler(object):
                                                        order, already_seen))
             return results
         targets, alias, _ = self.query.trim_joins(targets, joins, path)
-        return [(t.get_col(alias), order, False) for t in targets]
+        return [(OrderBy(t.get_col(alias), descending=descending), False) for t in targets]
 
     def _setup_joins(self, pieces, opts, alias):
         """
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index a613d8eba4..5d4538b2d9 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -1691,14 +1691,14 @@ class Query(object):
         """
         Adds items from the 'ordering' sequence to the query's "order by"
         clause. These items are either field names (not column names) --
-        possibly with a direction prefix ('-' or '?') -- or ordinals,
-        corresponding to column positions in the 'select' list.
+        possibly with a direction prefix ('-' or '?') -- or OrderBy
+        expressions.
 
         If 'ordering' is empty, all ordering is cleared from the query.
         """
         errors = []
         for item in ordering:
-            if not ORDER_PATTERN.match(item):
+            if not hasattr(item, 'resolve_expression') and not ORDER_PATTERN.match(item):
                 errors.append(item)
         if errors:
             raise FieldError('Invalid order_by arguments: %s' % errors)