From 268ed9cd8adfda0eb789a9f6c58c91684b4567d9 Mon Sep 17 00:00:00 2001 From: Simone Pellizzari Date: Sat, 6 Apr 2019 13:45:22 +0200 Subject: [PATCH] [2.2.x] Fixed #30332 -- Fixed crash of ordering by expressions with params in ArrayAgg and StringAgg. Backport of d0315584b5ed6a47b486e65f6c88f80189f337ef from master. --- django/contrib/postgres/aggregates/mixins.py | 16 ++++++++++------ docs/releases/2.2.1.txt | 5 +++++ tests/postgres_tests/test_aggregates.py | 9 +++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/django/contrib/postgres/aggregates/mixins.py b/django/contrib/postgres/aggregates/mixins.py index b270a5b653..6c1ce53b75 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 b24db37c1a..f7ac012fb7 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 85d6f45fd1..3622f6eff1 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.testcases import skipUnlessDBFeature from django.test.utils import Approximate @@ -38,6 +39,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): @@ -165,6 +172,8 @@ class TestGeneralAggregate(PostgreSQLTestCase): (F('char_field').desc(), 'Foo4;Foo3;Foo2;Foo1'), (F('char_field').asc(), 'Foo1;Foo2;Foo3;Foo4'), (F('char_field'), 'Foo1;Foo2;Foo3;Foo4'), + (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):