Fixed #33304 -- Allowed passing string expressions to Window(order_by).

4 files changed, 39 insertions, 29 deletions

diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py
index 07fc32d25b..afdf3402e5 100644
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -1333,11 +1333,13 @@ class Window(SQLiteNumericMixin, Expression):
 
         if self.order_by is not None:
             if isinstance(self.order_by, (list, tuple)):
-                self.order_by = ExpressionList(*self.order_by)
-            elif not isinstance(self.order_by, BaseExpression):
+                self.order_by = OrderByList(*self.order_by)
+            elif isinstance(self.order_by, (BaseExpression, str)):
+                self.order_by = OrderByList(self.order_by)
+            else:
                 raise ValueError(
-                    'order_by must be either an Expression or a sequence of '
-                    'expressions.'
+                    'Window.order_by must be either a string reference to a '
+                    'field, an expression, or a list or tuple of them.'
                 )
         super().__init__(output_field=output_field)
         self.source_expression = self._parse_expressions(expression)[0]
@@ -1363,18 +1365,17 @@ class Window(SQLiteNumericMixin, Expression):
                 compiler=compiler, connection=connection,
                 template='PARTITION BY %(expressions)s',
             )
-            window_sql.extend(sql_expr)
+            window_sql.append(sql_expr)
             window_params.extend(sql_params)
 
         if self.order_by is not None:
-            window_sql.append(' ORDER BY ')
             order_sql, order_params = compiler.compile(self.order_by)
-            window_sql.extend(order_sql)
+            window_sql.append(order_sql)
             window_params.extend(order_params)
 
         if self.frame:
             frame_sql, frame_params = compiler.compile(self.frame)
-            window_sql.append(' ' + frame_sql)
+            window_sql.append(frame_sql)
             window_params.extend(frame_params)
 
         params.extend(window_params)
@@ -1382,7 +1383,7 @@ class Window(SQLiteNumericMixin, Expression):
 
         return template % {
             'expression': expr_sql,
-            'window': ''.join(window_sql).strip()
+            'window': ' '.join(window_sql).strip()
         }, params
 
     def as_sqlite(self, compiler, connection):
@@ -1399,7 +1400,7 @@ class Window(SQLiteNumericMixin, Expression):
         return '{} OVER ({}{}{})'.format(
             str(self.source_expression),
             'PARTITION BY ' + str(self.partition_by) if self.partition_by else '',
-            'ORDER BY ' + str(self.order_by) if self.order_by else '',
+            str(self.order_by or ''),
             str(self.frame or ''),
         )
 
diff --git a/docs/ref/models/expressions.txt b/docs/ref/models/expressions.txt
index f938213bdd..5ea7f9f0aa 100644
--- a/docs/ref/models/expressions.txt
+++ b/docs/ref/models/expressions.txt
@@ -772,26 +772,31 @@ compute the result set.
 
 The ``output_field`` is specified either as an argument or by the expression.
 
-The ``order_by`` argument accepts an expression or a sequence of expressions on
-which you can call :meth:`~django.db.models.Expression.asc` and
-:meth:`~django.db.models.Expression.desc`. The ordering controls the order in
-which the expression is applied. For example, if you sum over the rows in a
-partition, the first result is the value of the first row, the second is the
-sum of first and second row.
+The ``order_by`` argument accepts an expression on which you can call
+:meth:`~django.db.models.Expression.asc` and
+:meth:`~django.db.models.Expression.desc`, a string of a field name (with an
+optional ``"-"`` prefix which indicates descending order), or a tuple or list
+of strings and/or expressions. The ordering controls the order in which the
+expression is applied. For example, if you sum over the rows in a partition,
+the first result is the value of the first row, the second is the sum of first
+and second row.
 
 The ``frame`` parameter specifies which other rows that should be used in the
 computation. See :ref:`window-frames` for details.
 
+.. versionchanged:: 4.1
+
+    Support for ``order_by`` by field name references was added.
+
 For example, to annotate each movie with the average rating for the movies by
 the same studio in the same genre and release year::
 
     >>> from django.db.models import Avg, F, Window
-    >>> from django.db.models.functions import ExtractYear
     >>> Movie.objects.annotate(
     >>>     avg_rating=Window(
     >>>         expression=Avg('rating'),
     >>>         partition_by=[F('studio'), F('genre')],
-    >>>         order_by=ExtractYear('released').asc(),
+    >>>         order_by='released__year',
     >>>     ),
     >>> )
 
@@ -805,10 +810,9 @@ partition and ordering from the previous example is extracted into a dictionary
 to reduce repetition::
 
     >>> from django.db.models import Avg, F, Max, Min, Window
-    >>> from django.db.models.functions import ExtractYear
     >>> window = {
     >>>    'partition_by': [F('studio'), F('genre')],
-    >>>    'order_by': ExtractYear('released').asc(),
+    >>>    'order_by': 'released__year',
     >>> }
     >>> Movie.objects.annotate(
     >>>     avg_rating=Window(
@@ -887,12 +891,11 @@ same genre in the same year, this ``RowRange`` example annotates each movie
 with the average rating of a movie's two prior and two following peers::
 
     >>> from django.db.models import Avg, F, RowRange, Window
-    >>> from django.db.models.functions import ExtractYear
     >>> Movie.objects.annotate(
     >>>     avg_rating=Window(
     >>>         expression=Avg('rating'),
     >>>         partition_by=[F('studio'), F('genre')],
-    >>>         order_by=ExtractYear('released').asc(),
+    >>>         order_by='released__year',
     >>>         frame=RowRange(start=-2, end=2),
     >>>     ),
     >>> )
@@ -901,14 +904,14 @@ If the database supports it, you can specify the start and end points based on
 values of an expression in the partition. If the ``released`` field of the
 ``Movie`` model stores the release month of each movies, this ``ValueRange``
 example annotates each movie with the average rating of a movie's peers
-released between twelve months before and twelve months after the each movie.
+released between twelve months before and twelve months after the each movie::
 
     >>> from django.db.models import Avg, F, ValueRange, Window
     >>> Movie.objects.annotate(
     >>>     avg_rating=Window(
     >>>         expression=Avg('rating'),
     >>>         partition_by=[F('studio'), F('genre')],
-    >>>         order_by=F('released').asc(),
+    >>>         order_by='released__year',
     >>>         frame=ValueRange(start=-12, end=12),
     >>>     ),
     >>> )
diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt
index e8296e4d36..9998802105 100644
--- a/docs/releases/4.1.txt
+++ b/docs/releases/4.1.txt
@@ -185,7 +185,9 @@ Migrations
 Models
 ~~~~~~
 
-* ...
+* The ``order_by`` argument of the
+  :class:`~django.db.models.expressions.Window` expression now accepts string
+  references to fields and transforms.
 
 Requests and Responses
 ~~~~~~~~~~~~~~~~~~~~~~
diff --git a/tests/expressions_window/tests.py b/tests/expressions_window/tests.py
index 8b1b4a8a3f..5bd31b8191 100644
--- a/tests/expressions_window/tests.py
+++ b/tests/expressions_window/tests.py
@@ -51,6 +51,7 @@ class WindowFunctionTests(TestCase):
         tests = [
             ExtractYear(F('hire_date')).asc(),
             F('hire_date__year').asc(),
+            'hire_date__year',
         ]
         for order_by in tests:
             with self.subTest(order_by=order_by):
@@ -473,7 +474,7 @@ class WindowFunctionTests(TestCase):
         """
         qs = Employee.objects.annotate(ntile=Window(
             expression=Ntile(num_buckets=4),
-            order_by=F('salary').desc(),
+            order_by='-salary',
         )).order_by('ntile', '-salary', 'name')
         self.assertQuerysetEqual(qs, [
             ('Miller', 'Management', 100000, 1),
@@ -875,7 +876,7 @@ class NonQueryWindowTests(SimpleTestCase):
         )
         self.assertEqual(
             repr(Window(expression=Avg('salary'), order_by=F('department').asc())),
-            '<Window: Avg(F(salary)) OVER (ORDER BY OrderBy(F(department), descending=False))>'
+            '<Window: Avg(F(salary)) OVER (OrderByList(OrderBy(F(department), descending=False)))>'
         )
 
     def test_window_frame_repr(self):
@@ -942,9 +943,12 @@ class NonQueryWindowTests(SimpleTestCase):
             qs.filter(equal=True)
 
     def test_invalid_order_by(self):
-        msg = 'order_by must be either an Expression or a sequence of expressions'
+        msg = (
+            'Window.order_by must be either a string reference to a field, an '
+            'expression, or a list or tuple of them.'
+        )
         with self.assertRaisesMessage(ValueError, msg):
-            Window(expression=Sum('power'), order_by='-horse')
+            Window(expression=Sum('power'), order_by={'-horse'})
 
     def test_invalid_source_expression(self):
         msg = "Expression 'Upper' isn't compatible with OVER clauses."