diff --git a/django/contrib/postgres/aggregates/mixins.py b/django/contrib/postgres/aggregates/mixins.py index b270a5b653c..6c1ce53b75c 100644 --- a/django/contrib/postgres/aggregates/mixins.py +++ b/django/contrib/postgres/aggregates/mixins.py @@ -21,13 +21,17 @@ class OrderableAggMixin: def as_sql(self, compiler, connection): if self.ordering: - self.extra['ordering'] = 'ORDER BY ' + ', '.join(( - ordering_element.as_sql(compiler, connection)[0] - for ordering_element in self.ordering + ordering_params = [] + ordering_expr_sql = [] + for expr in self.ordering: + expr_sql, expr_params = expr.as_sql(compiler, connection) + ordering_expr_sql.append(expr_sql) + ordering_params.extend(expr_params) + sql, sql_params = super().as_sql(compiler, connection, ordering=( + 'ORDER BY ' + ', '.join(ordering_expr_sql) )) - else: - self.extra['ordering'] = '' - return super().as_sql(compiler, connection) + return sql, sql_params + ordering_params + return super().as_sql(compiler, connection, ordering='') def get_source_expressions(self): return self.source_expressions + self.ordering diff --git a/docs/releases/2.2.1.txt b/docs/releases/2.2.1.txt index b24db37c1a3..f7ac012fb7f 100644 --- a/docs/releases/2.2.1.txt +++ b/docs/releases/2.2.1.txt @@ -17,3 +17,8 @@ Bugfixes * Fixed a regression in Django 2.2 that caused a crash when loading the template for the technical 500 debug page (:ticket:`30324`). + +* Fixed crash of ``ordering`` argument in + :class:`~django.contrib.postgres.aggregates.ArrayAgg` and + :class:`~django.contrib.postgres.aggregates.StringAgg` when it contains an + expression with params (:ticket:`30332`). diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py index 6b77787dcce..23f3c59ded1 100644 --- a/tests/postgres_tests/test_aggregates.py +++ b/tests/postgres_tests/test_aggregates.py @@ -1,6 +1,7 @@ import json from django.db.models.expressions import F, Value +from django.db.models.functions import Concat, Substr from django.test.utils import Approximate from . import PostgreSQLTestCase @@ -37,6 +38,12 @@ class TestGeneralAggregate(PostgreSQLTestCase): ((F('boolean_field'), F('char_field').desc()), ['Foo4', 'Foo2', 'Foo3', 'Foo1']), ('char_field', ['Foo1', 'Foo2', 'Foo3', 'Foo4']), ('-char_field', ['Foo4', 'Foo3', 'Foo2', 'Foo1']), + (Concat('char_field', Value('@')), ['Foo1', 'Foo2', 'Foo3', 'Foo4']), + (Concat('char_field', Value('@')).desc(), ['Foo4', 'Foo3', 'Foo2', 'Foo1']), + ( + (Substr('char_field', 1, 1), F('integer_field'), Substr('char_field', 4, 1).desc()), + ['Foo3', 'Foo1', 'Foo2', 'Foo4'], + ), ) for ordering, expected_output in ordering_test_cases: with self.subTest(ordering=ordering, expected_output=expected_output): @@ -166,6 +173,8 @@ class TestGeneralAggregate(PostgreSQLTestCase): (F('char_field'), 'Foo1;Foo2;Foo3;Foo4'), ('char_field', 'Foo1;Foo2;Foo3;Foo4'), ('-char_field', 'Foo4;Foo3;Foo2;Foo1'), + (Concat('char_field', Value('@')), 'Foo1;Foo2;Foo3;Foo4'), + (Concat('char_field', Value('@')).desc(), 'Foo4;Foo3;Foo2;Foo1'), ) for ordering, expected_output in ordering_test_cases: with self.subTest(ordering=ordering, expected_output=expected_output):