From d0315584b5ed6a47b486e65f6c88f80189f337ef Mon Sep 17 00:00:00 2001
From: Simone Pellizzari <simone6021@libero.it>
Date: Sat, 6 Apr 2019 13:45:22 +0200
Subject: [PATCH] Fixed #30332 -- Fixed crash of ordering by expressions with
params in ArrayAgg and StringAgg.
---
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 6b77787dcc..23f3c59ded 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):