From c8df2f994130d74ec35d32a36e30aad7d6ea8e3a Mon Sep 17 00:00:00 2001 From: Chris Muthig Date: Wed, 3 Apr 2024 16:09:44 -0600 Subject: [PATCH] Fixed #35339 -- Fixed PostgreSQL aggregate's filter and order_by params order. Updated OrderableAggMixin.as_sql() to separate the order_by parameters from the filter parameters. Previously, the parameters and SQL were calculated by the Aggregate parent class, resulting in a mixture of order_by and filter parameters. Thanks Simon Charette for the review. --- django/contrib/postgres/aggregates/mixins.py | 27 +++++++++++++++----- tests/postgres_tests/test_aggregates.py | 12 ++++++++- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/django/contrib/postgres/aggregates/mixins.py b/django/contrib/postgres/aggregates/mixins.py index 527626da4c144..d6ff535158dba 100644 --- a/django/contrib/postgres/aggregates/mixins.py +++ b/django/contrib/postgres/aggregates/mixins.py @@ -1,3 +1,4 @@ +from django.core.exceptions import FullResultSet from django.db.models.expressions import OrderByList @@ -24,9 +25,23 @@ def set_source_expressions(self, exprs): return super().set_source_expressions(exprs) def as_sql(self, compiler, connection): - if self.order_by is not None: - order_by_sql, order_by_params = compiler.compile(self.order_by) - else: - order_by_sql, order_by_params = "", () - sql, sql_params = super().as_sql(compiler, connection, ordering=order_by_sql) - return sql, (*sql_params, *order_by_params) + *source_exprs, filtering_expr, ordering_expr = self.get_source_expressions() + + order_by_sql = "" + order_by_params = [] + if ordering_expr is not None: + order_by_sql, order_by_params = compiler.compile(ordering_expr) + + filter_params = [] + if filtering_expr is not None: + try: + _, filter_params = compiler.compile(filtering_expr) + except FullResultSet: + pass + + source_params = [] + for source_expr in source_exprs: + source_params += compiler.compile(source_expr)[1] + + sql, _ = super().as_sql(compiler, connection, ordering=order_by_sql) + return sql, (*source_params, *order_by_params, *filter_params) diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py index 386c55da25500..7e1e16d0c0db2 100644 --- a/tests/postgres_tests/test_aggregates.py +++ b/tests/postgres_tests/test_aggregates.py @@ -12,7 +12,7 @@ Window, ) from django.db.models.fields.json import KeyTextTransform, KeyTransform -from django.db.models.functions import Cast, Concat, Substr +from django.db.models.functions import Cast, Concat, LPad, Substr from django.test import skipUnlessDBFeature from django.test.utils import Approximate from django.utils import timezone @@ -238,6 +238,16 @@ def test_array_agg_jsonfield_ordering(self): ) self.assertEqual(values, {"arrayagg": ["en", "pl"]}) + def test_array_agg_filter_and_ordering_params(self): + values = AggregateTestModel.objects.aggregate( + arrayagg=ArrayAgg( + "char_field", + filter=Q(json_field__has_key="lang"), + ordering=LPad(Cast("integer_field", CharField()), 2, Value("0")), + ) + ) + self.assertEqual(values, {"arrayagg": ["Foo2", "Foo4"]}) + def test_array_agg_filter(self): values = AggregateTestModel.objects.aggregate( arrayagg=ArrayAgg("integer_field", filter=Q(integer_field__gt=0)),