mirror of
				https://github.com/django/django.git
				synced 2025-10-31 01:25:32 +00:00 
			
		
		
		
	Fixed #33304 -- Allowed passing string expressions to Window(order_by).
This commit is contained in:
		
				
					committed by
					
						 Mariusz Felisiak
						Mariusz Felisiak
					
				
			
			
				
	
			
			
			
						parent
						
							e06dc4571e
						
					
				
				
					commit
					aec71aaa5b
				
			| @@ -1333,11 +1333,13 @@ class Window(SQLiteNumericMixin, Expression): | |||||||
|  |  | ||||||
|         if self.order_by is not None: |         if self.order_by is not None: | ||||||
|             if isinstance(self.order_by, (list, tuple)): |             if isinstance(self.order_by, (list, tuple)): | ||||||
|                 self.order_by = ExpressionList(*self.order_by) |                 self.order_by = OrderByList(*self.order_by) | ||||||
|             elif not isinstance(self.order_by, BaseExpression): |             elif isinstance(self.order_by, (BaseExpression, str)): | ||||||
|  |                 self.order_by = OrderByList(self.order_by) | ||||||
|  |             else: | ||||||
|                 raise ValueError( |                 raise ValueError( | ||||||
|                     'order_by must be either an Expression or a sequence of ' |                     'Window.order_by must be either a string reference to a ' | ||||||
|                     'expressions.' |                     'field, an expression, or a list or tuple of them.' | ||||||
|                 ) |                 ) | ||||||
|         super().__init__(output_field=output_field) |         super().__init__(output_field=output_field) | ||||||
|         self.source_expression = self._parse_expressions(expression)[0] |         self.source_expression = self._parse_expressions(expression)[0] | ||||||
| @@ -1363,18 +1365,17 @@ class Window(SQLiteNumericMixin, Expression): | |||||||
|                 compiler=compiler, connection=connection, |                 compiler=compiler, connection=connection, | ||||||
|                 template='PARTITION BY %(expressions)s', |                 template='PARTITION BY %(expressions)s', | ||||||
|             ) |             ) | ||||||
|             window_sql.extend(sql_expr) |             window_sql.append(sql_expr) | ||||||
|             window_params.extend(sql_params) |             window_params.extend(sql_params) | ||||||
|  |  | ||||||
|         if self.order_by is not None: |         if self.order_by is not None: | ||||||
|             window_sql.append(' ORDER BY ') |  | ||||||
|             order_sql, order_params = compiler.compile(self.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) |             window_params.extend(order_params) | ||||||
|  |  | ||||||
|         if self.frame: |         if self.frame: | ||||||
|             frame_sql, frame_params = compiler.compile(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) |             window_params.extend(frame_params) | ||||||
|  |  | ||||||
|         params.extend(window_params) |         params.extend(window_params) | ||||||
| @@ -1382,7 +1383,7 @@ class Window(SQLiteNumericMixin, Expression): | |||||||
|  |  | ||||||
|         return template % { |         return template % { | ||||||
|             'expression': expr_sql, |             'expression': expr_sql, | ||||||
|             'window': ''.join(window_sql).strip() |             'window': ' '.join(window_sql).strip() | ||||||
|         }, params |         }, params | ||||||
|  |  | ||||||
|     def as_sqlite(self, compiler, connection): |     def as_sqlite(self, compiler, connection): | ||||||
| @@ -1399,7 +1400,7 @@ class Window(SQLiteNumericMixin, Expression): | |||||||
|         return '{} OVER ({}{}{})'.format( |         return '{} OVER ({}{}{})'.format( | ||||||
|             str(self.source_expression), |             str(self.source_expression), | ||||||
|             'PARTITION BY ' + str(self.partition_by) if self.partition_by else '', |             '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 ''), |             str(self.frame or ''), | ||||||
|         ) |         ) | ||||||
|  |  | ||||||
|   | |||||||
| @@ -772,26 +772,31 @@ compute the result set. | |||||||
|  |  | ||||||
| The ``output_field`` is specified either as an argument or by the expression. | 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 | The ``order_by`` argument accepts an expression on which you can call | ||||||
| which you can call :meth:`~django.db.models.Expression.asc` and | :meth:`~django.db.models.Expression.asc` and | ||||||
| :meth:`~django.db.models.Expression.desc`. The ordering controls the order in | :meth:`~django.db.models.Expression.desc`, a string of a field name (with an | ||||||
| which the expression is applied. For example, if you sum over the rows in a | optional ``"-"`` prefix which indicates descending order), or a tuple or list | ||||||
| partition, the first result is the value of the first row, the second is the | of strings and/or expressions. The ordering controls the order in which the | ||||||
| sum of first and second row. | 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 | The ``frame`` parameter specifies which other rows that should be used in the | ||||||
| computation. See :ref:`window-frames` for details. | 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 | For example, to annotate each movie with the average rating for the movies by | ||||||
| the same studio in the same genre and release year:: | the same studio in the same genre and release year:: | ||||||
|  |  | ||||||
|     >>> from django.db.models import Avg, F, Window |     >>> from django.db.models import Avg, F, Window | ||||||
|     >>> from django.db.models.functions import ExtractYear |  | ||||||
|     >>> Movie.objects.annotate( |     >>> Movie.objects.annotate( | ||||||
|     >>>     avg_rating=Window( |     >>>     avg_rating=Window( | ||||||
|     >>>         expression=Avg('rating'), |     >>>         expression=Avg('rating'), | ||||||
|     >>>         partition_by=[F('studio'), F('genre')], |     >>>         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:: | to reduce repetition:: | ||||||
|  |  | ||||||
|     >>> from django.db.models import Avg, F, Max, Min, Window |     >>> from django.db.models import Avg, F, Max, Min, Window | ||||||
|     >>> from django.db.models.functions import ExtractYear |  | ||||||
|     >>> window = { |     >>> window = { | ||||||
|     >>>    'partition_by': [F('studio'), F('genre')], |     >>>    'partition_by': [F('studio'), F('genre')], | ||||||
|     >>>    'order_by': ExtractYear('released').asc(), |     >>>    'order_by': 'released__year', | ||||||
|     >>> } |     >>> } | ||||||
|     >>> Movie.objects.annotate( |     >>> Movie.objects.annotate( | ||||||
|     >>>     avg_rating=Window( |     >>>     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:: | 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 import Avg, F, RowRange, Window | ||||||
|     >>> from django.db.models.functions import ExtractYear |  | ||||||
|     >>> Movie.objects.annotate( |     >>> Movie.objects.annotate( | ||||||
|     >>>     avg_rating=Window( |     >>>     avg_rating=Window( | ||||||
|     >>>         expression=Avg('rating'), |     >>>         expression=Avg('rating'), | ||||||
|     >>>         partition_by=[F('studio'), F('genre')], |     >>>         partition_by=[F('studio'), F('genre')], | ||||||
|     >>>         order_by=ExtractYear('released').asc(), |     >>>         order_by='released__year', | ||||||
|     >>>         frame=RowRange(start=-2, end=2), |     >>>         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 | values of an expression in the partition. If the ``released`` field of the | ||||||
| ``Movie`` model stores the release month of each movies, this ``ValueRange`` | ``Movie`` model stores the release month of each movies, this ``ValueRange`` | ||||||
| example annotates each movie with the average rating of a movie's peers | 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 |     >>> from django.db.models import Avg, F, ValueRange, Window | ||||||
|     >>> Movie.objects.annotate( |     >>> Movie.objects.annotate( | ||||||
|     >>>     avg_rating=Window( |     >>>     avg_rating=Window( | ||||||
|     >>>         expression=Avg('rating'), |     >>>         expression=Avg('rating'), | ||||||
|     >>>         partition_by=[F('studio'), F('genre')], |     >>>         partition_by=[F('studio'), F('genre')], | ||||||
|     >>>         order_by=F('released').asc(), |     >>>         order_by='released__year', | ||||||
|     >>>         frame=ValueRange(start=-12, end=12), |     >>>         frame=ValueRange(start=-12, end=12), | ||||||
|     >>>     ), |     >>>     ), | ||||||
|     >>> ) |     >>> ) | ||||||
|   | |||||||
| @@ -185,7 +185,9 @@ Migrations | |||||||
| Models | 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 | Requests and Responses | ||||||
| ~~~~~~~~~~~~~~~~~~~~~~ | ~~~~~~~~~~~~~~~~~~~~~~ | ||||||
|   | |||||||
| @@ -51,6 +51,7 @@ class WindowFunctionTests(TestCase): | |||||||
|         tests = [ |         tests = [ | ||||||
|             ExtractYear(F('hire_date')).asc(), |             ExtractYear(F('hire_date')).asc(), | ||||||
|             F('hire_date__year').asc(), |             F('hire_date__year').asc(), | ||||||
|  |             'hire_date__year', | ||||||
|         ] |         ] | ||||||
|         for order_by in tests: |         for order_by in tests: | ||||||
|             with self.subTest(order_by=order_by): |             with self.subTest(order_by=order_by): | ||||||
| @@ -473,7 +474,7 @@ class WindowFunctionTests(TestCase): | |||||||
|         """ |         """ | ||||||
|         qs = Employee.objects.annotate(ntile=Window( |         qs = Employee.objects.annotate(ntile=Window( | ||||||
|             expression=Ntile(num_buckets=4), |             expression=Ntile(num_buckets=4), | ||||||
|             order_by=F('salary').desc(), |             order_by='-salary', | ||||||
|         )).order_by('ntile', '-salary', 'name') |         )).order_by('ntile', '-salary', 'name') | ||||||
|         self.assertQuerysetEqual(qs, [ |         self.assertQuerysetEqual(qs, [ | ||||||
|             ('Miller', 'Management', 100000, 1), |             ('Miller', 'Management', 100000, 1), | ||||||
| @@ -875,7 +876,7 @@ class NonQueryWindowTests(SimpleTestCase): | |||||||
|         ) |         ) | ||||||
|         self.assertEqual( |         self.assertEqual( | ||||||
|             repr(Window(expression=Avg('salary'), order_by=F('department').asc())), |             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): |     def test_window_frame_repr(self): | ||||||
| @@ -942,9 +943,12 @@ class NonQueryWindowTests(SimpleTestCase): | |||||||
|             qs.filter(equal=True) |             qs.filter(equal=True) | ||||||
|  |  | ||||||
|     def test_invalid_order_by(self): |     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): |         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): |     def test_invalid_source_expression(self): | ||||||
|         msg = "Expression 'Upper' isn't compatible with OVER clauses." |         msg = "Expression 'Upper' isn't compatible with OVER clauses." | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user