Refs #35444 -- Deprecated contrib.postgres aggregates ordering for order_by.

Aligns the argument with SQL standards already used in Window.order_by and
sets up for adding support to Aggregate.

django/contrib/postgres/aggregates/general.py

@@ -17,7 +17,7 @@ __all__ = [
 
 class ArrayAgg(OrderableAggMixin, Aggregate):
     function = "ARRAY_AGG"
-    template = "%(function)s(%(distinct)s%(expressions)s %(ordering)s)"
+    template = "%(function)s(%(distinct)s%(expressions)s %(order_by)s)"
     allow_distinct = True
 
     @property
@@ -49,14 +49,14 @@ class BoolOr(Aggregate):
 
 class JSONBAgg(OrderableAggMixin, Aggregate):
     function = "JSONB_AGG"
-    template = "%(function)s(%(distinct)s%(expressions)s %(ordering)s)"
+    template = "%(function)s(%(distinct)s%(expressions)s %(order_by)s)"
     allow_distinct = True
     output_field = JSONField()
 
 
 class StringAgg(OrderableAggMixin, Aggregate):
     function = "STRING_AGG"
-    template = "%(function)s(%(distinct)s%(expressions)s %(ordering)s)"
+    template = "%(function)s(%(distinct)s%(expressions)s %(order_by)s)"
     allow_distinct = True
     output_field = TextField()
 

django/contrib/postgres/aggregates/mixins.py

@@ -1,15 +1,30 @@
+import warnings
+
 from django.core.exceptions import FullResultSet
 from django.db.models.expressions import OrderByList
+from django.utils.deprecation import RemovedInDjango61Warning
 
 
 class OrderableAggMixin:
-    def __init__(self, *expressions, ordering=(), **extra):
+    # RemovedInDjango61Warning: When the deprecation ends, replace with:
-        if not ordering:
+    # def __init__(self, *expressions, order_by=(), **extra):
+    def __init__(self, *expressions, ordering=(), order_by=(), **extra):
+        # RemovedInDjango61Warning.
+        if ordering:
+            warnings.warn(
+                "The ordering argument is deprecated. Use order_by instead.",
+                category=RemovedInDjango61Warning,
+                stacklevel=2,
+            )
+            if order_by:
+                raise TypeError("Cannot specify both order_by and ordering.")
+            order_by = ordering
+        if not order_by:
             self.order_by = None
-        elif isinstance(ordering, (list, tuple)):
+        elif isinstance(order_by, (list, tuple)):
-            self.order_by = OrderByList(*ordering)
+            self.order_by = OrderByList(*order_by)
         else:
-            self.order_by = OrderByList(ordering)
+            self.order_by = OrderByList(order_by)
         super().__init__(*expressions, **extra)
 
     def resolve_expression(self, *args, **kwargs):
@@ -25,12 +40,12 @@ class OrderableAggMixin:
         return super().set_source_expressions(exprs)
 
     def as_sql(self, compiler, connection):
-        *source_exprs, filtering_expr, ordering_expr = self.get_source_expressions()
+        *source_exprs, filtering_expr, order_by_expr = self.get_source_expressions()
 
         order_by_sql = ""
         order_by_params = []
-        if ordering_expr is not None:
+        if order_by_expr is not None:
-            order_by_sql, order_by_params = compiler.compile(ordering_expr)
+            order_by_sql, order_by_params = compiler.compile(order_by_expr)
 
         filter_params = []
         if filtering_expr is not None:
@@ -43,5 +58,5 @@ class OrderableAggMixin:
         for source_expr in source_exprs:
             source_params += compiler.compile(source_expr)[1]
 
-        sql, _ = super().as_sql(compiler, connection, ordering=order_by_sql)
+        sql, _ = super().as_sql(compiler, connection, order_by=order_by_sql)
         return sql, (*source_params, *order_by_params, *filter_params)

docs/internals/deprecation.txt

@@ -22,6 +22,11 @@ details on these changes.
   ``django.contrib.auth.login()`` and ``django.contrib.auth.alogin()`` will be
   removed.
 
+* The ``ordering`` keyword argument of the PostgreSQL specific aggregation
+  functions ``django.contrib.postgres.aggregates.ArrayAgg``,
+  ``django.contrib.postgres.aggregates.JSONBAgg``, and
+  ``django.contrib.postgres.aggregates.StringAgg`` will be removed.
+
 .. _deprecation-removed-in-6.0:
 
 6.0

docs/ref/contrib/postgres/aggregates.txt

@@ -30,7 +30,7 @@ General-purpose aggregation functions
 ``ArrayAgg``
 ------------
 
-.. class:: ArrayAgg(expression, distinct=False, filter=None, default=None, ordering=(), **extra)
+.. class:: ArrayAgg(expression, distinct=False, filter=None, default=None, order_by=(), **extra)
 
     Returns a list of values, including nulls, concatenated into an array, or
     ``default`` if there are no values.
@@ -40,7 +40,9 @@ General-purpose aggregation functions
         An optional boolean argument that determines if array values
         will be distinct. Defaults to ``False``.
 
-    .. attribute:: ordering
+    .. attribute:: order_by
+
+        .. versionadded:: 5.2
 
         An optional string of a field name (with an optional ``"-"`` prefix
         which indicates descending order) or an expression (or a tuple or list
@@ -55,6 +57,11 @@ General-purpose aggregation functions
 
             F("some_field").desc()
 
+    .. deprecated:: 5.2
+
+        The ``ordering`` keyword argument is deprecated. Use
+        :attr:`ArrayAgg.order_by` instead.
+
 ``BitAnd``
 ----------
 
@@ -130,7 +137,7 @@ General-purpose aggregation functions
 ``JSONBAgg``
 ------------
 
-.. class:: JSONBAgg(expressions, distinct=False, filter=None, default=None, ordering=(), **extra)
+.. class:: JSONBAgg(expressions, distinct=False, filter=None, default=None, order_by=(), **extra)
 
     Returns the input values as a ``JSON`` array, or ``default`` if there are
     no values. You can query the result using :lookup:`key and index lookups
@@ -141,14 +148,16 @@ General-purpose aggregation functions
         An optional boolean argument that determines if array values will be
         distinct. Defaults to ``False``.
 
-    .. attribute:: ordering
+    .. attribute:: order_by
+
+        .. versionadded:: 5.2
 
         An optional string of a field name (with an optional ``"-"`` prefix
         which indicates descending order) or an expression (or a tuple or list
         of strings and/or expressions) that specifies the ordering of the
         elements in the result list.
 
-        Examples are the same as for :attr:`ArrayAgg.ordering`.
+        Examples are the same as for :attr:`ArrayAgg.order_by`.
 
     Usage example::
 
@@ -168,7 +177,7 @@ General-purpose aggregation functions
         >>> Room.objects.annotate(
         ...     requirements=JSONBAgg(
         ...         "hotelreservation__requirements",
-        ...         ordering="-hotelreservation__start",
+        ...         order_by="-hotelreservation__start",
         ...     )
         ... ).filter(requirements__0__sea_view=True).values("number", "requirements")
         <QuerySet [{'number': 102, 'requirements': [
@@ -176,10 +185,15 @@ General-purpose aggregation functions
             {'parking': True, 'double_bed': True}
         ]}]>
 
+    .. deprecated:: 5.2
+
+        The ``ordering`` keyword argument is deprecated. Use
+        :attr:`JSONBAgg.order_by` instead.
+
 ``StringAgg``
 -------------
 
-.. class:: StringAgg(expression, delimiter, distinct=False, filter=None, default=None, ordering=())
+.. class:: StringAgg(expression, delimiter, distinct=False, filter=None, default=None, order_by=())
 
     Returns the input values concatenated into a string, separated by
     the ``delimiter`` string, or ``default`` if there are no values.
@@ -193,14 +207,16 @@ General-purpose aggregation functions
         An optional boolean argument that determines if concatenated values
         will be distinct. Defaults to ``False``.
 
-    .. attribute:: ordering
+    .. attribute:: order_by
+
+        .. versionadded:: 5.2
 
         An optional string of a field name (with an optional ``"-"`` prefix
         which indicates descending order) or an expression (or a tuple or list
         of strings and/or expressions) that specifies the ordering of the
         elements in the result string.
 
-        Examples are the same as for :attr:`ArrayAgg.ordering`.
+        Examples are the same as for :attr:`ArrayAgg.order_by`.
 
     Usage example::
 
@@ -224,13 +240,18 @@ General-purpose aggregation functions
         ...     publication_names=StringAgg(
         ...         "publications__title",
         ...         delimiter=", ",
-        ...         ordering="publications__title",
+        ...         order_by="publications__title",
         ...     )
         ... ).values("headline", "publication_names")
         <QuerySet [{
             'headline': 'NASA uses Python', 'publication_names': 'Science News, The Python Journal'
         }]>
 
+    .. deprecated:: 5.2
+
+        The ``ordering`` keyword argument is deprecated. Use
+        :attr:`StringAgg.order_by` instead.
+
 Aggregate functions for statistics
 ==================================
 

docs/releases/3.2.txt

@@ -250,7 +250,7 @@ Minor features
 * The new ``ExclusionConstraint.opclasses`` attribute allows setting PostgreSQL
   operator classes.
 
-* The new :attr:`.JSONBAgg.ordering` attribute determines the ordering of the
+* The new ``JSONBAgg.ordering`` attribute determines the ordering of the
   aggregated elements.
 
 * The new :attr:`.JSONBAgg.distinct` attribute determines if aggregated values

docs/releases/5.2.txt

@@ -495,3 +495,9 @@ Miscellaneous
 * The fallback to ``request.user`` when ``user`` is ``None`` in
   ``django.contrib.auth.login()`` and ``django.contrib.auth.alogin()`` will be
   removed.
+
+* The ``ordering`` keyword argument of the PostgreSQL specific aggregation
+  functions ``django.contrib.postgres.aggregates.ArrayAgg``,
+  ``django.contrib.postgres.aggregates.JSONBAgg``, and
+  ``django.contrib.postgres.aggregates.StringAgg`` is deprecated in favor
+  of the ``order_by`` argument.

tests/postgres_tests/test_aggregates.py

@@ -15,6 +15,7 @@ from django.db.models.fields.json import KeyTextTransform, KeyTransform
 from django.db.models.functions import Cast, Concat, LPad, Substr
 from django.test.utils import Approximate
 from django.utils import timezone
+from django.utils.deprecation import RemovedInDjango61Warning
 
 from . import PostgreSQLTestCase
 from .models import AggregateTestModel, HotelReservation, Room, StatTestModel
@@ -148,12 +149,36 @@ class TestGeneralAggregate(PostgreSQLTestCase):
                     )
                     self.assertEqual(values, {"aggregation": expected_result})
 
+    def test_ordering_warns_of_deprecation(self):
+        msg = "The ordering argument is deprecated. Use order_by instead."
+        with self.assertWarnsMessage(RemovedInDjango61Warning, msg) as ctx:
+            values = AggregateTestModel.objects.aggregate(
+                arrayagg=ArrayAgg("integer_field", ordering=F("integer_field").desc())
+            )
+            self.assertEqual(values, {"arrayagg": [2, 1, 0, 0]})
+        self.assertEqual(ctx.filename, __file__)
+
+    def test_ordering_and_order_by_causes_error(self):
+        with self.assertWarns(RemovedInDjango61Warning):
+            with self.assertRaisesMessage(
+                TypeError,
+                "Cannot specify both order_by and ordering.",
+            ):
+                AggregateTestModel.objects.aggregate(
+                    stringagg=StringAgg(
+                        "char_field",
+                        delimiter=Value("'"),
+                        order_by="char_field",
+                        ordering="char_field",
+                    )
+                )
+
     def test_array_agg_charfield(self):
         values = AggregateTestModel.objects.aggregate(arrayagg=ArrayAgg("char_field"))
         self.assertEqual(values, {"arrayagg": ["Foo1", "Foo2", "Foo4", "Foo3"]})
 
-    def test_array_agg_charfield_ordering(self):
+    def test_array_agg_charfield_order_by(self):
-        ordering_test_cases = (
+        order_by_test_cases = (
             (F("char_field").desc(), ["Foo4", "Foo3", "Foo2", "Foo1"]),
             (F("char_field").asc(), ["Foo1", "Foo2", "Foo3", "Foo4"]),
             (F("char_field"), ["Foo1", "Foo2", "Foo3", "Foo4"]),
@@ -178,10 +203,10 @@ class TestGeneralAggregate(PostgreSQLTestCase):
                 ["Foo3", "Foo1", "Foo2", "Foo4"],
             ),
         )
-        for ordering, expected_output in ordering_test_cases:
+        for order_by, expected_output in order_by_test_cases:
-            with self.subTest(ordering=ordering, expected_output=expected_output):
+            with self.subTest(order_by=order_by, expected_output=expected_output):
                 values = AggregateTestModel.objects.aggregate(
-                    arrayagg=ArrayAgg("char_field", ordering=ordering)
+                    arrayagg=ArrayAgg("char_field", order_by=order_by)
                 )
                 self.assertEqual(values, {"arrayagg": expected_output})
 
@@ -191,9 +216,9 @@ class TestGeneralAggregate(PostgreSQLTestCase):
         )
         self.assertEqual(values, {"arrayagg": [0, 1, 2, 0]})
 
-    def test_array_agg_integerfield_ordering(self):
+    def test_array_agg_integerfield_order_by(self):
         values = AggregateTestModel.objects.aggregate(
-            arrayagg=ArrayAgg("integer_field", ordering=F("integer_field").desc())
+            arrayagg=ArrayAgg("integer_field", order_by=F("integer_field").desc())
         )
         self.assertEqual(values, {"arrayagg": [2, 1, 0, 0]})
 
@@ -203,16 +228,16 @@ class TestGeneralAggregate(PostgreSQLTestCase):
         )
         self.assertEqual(values, {"arrayagg": [True, False, False, True]})
 
-    def test_array_agg_booleanfield_ordering(self):
+    def test_array_agg_booleanfield_order_by(self):
-        ordering_test_cases = (
+        order_by_test_cases = (
             (F("boolean_field").asc(), [False, False, True, True]),
             (F("boolean_field").desc(), [True, True, False, False]),
             (F("boolean_field"), [False, False, True, True]),
         )
-        for ordering, expected_output in ordering_test_cases:
+        for order_by, expected_output in order_by_test_cases:
-            with self.subTest(ordering=ordering, expected_output=expected_output):
+            with self.subTest(order_by=order_by, expected_output=expected_output):
                 values = AggregateTestModel.objects.aggregate(
-                    arrayagg=ArrayAgg("boolean_field", ordering=ordering)
+                    arrayagg=ArrayAgg("boolean_field", order_by=order_by)
                 )
                 self.assertEqual(values, {"arrayagg": expected_output})
 
@@ -225,22 +250,22 @@ class TestGeneralAggregate(PostgreSQLTestCase):
         )
         self.assertEqual(values, {"arrayagg": ["pl", "en"]})
 
-    def test_array_agg_jsonfield_ordering(self):
+    def test_array_agg_jsonfield_order_by(self):
         values = AggregateTestModel.objects.aggregate(
             arrayagg=ArrayAgg(
                 KeyTransform("lang", "json_field"),
                 filter=Q(json_field__lang__isnull=False),
-                ordering=KeyTransform("lang", "json_field"),
+                order_by=KeyTransform("lang", "json_field"),
             ),
         )
         self.assertEqual(values, {"arrayagg": ["en", "pl"]})
 
-    def test_array_agg_filter_and_ordering_params(self):
+    def test_array_agg_filter_and_order_by_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")),
+                order_by=LPad(Cast("integer_field", CharField()), 2, Value("0")),
             )
         )
         self.assertEqual(values, {"arrayagg": ["Foo2", "Foo4"]})
@@ -406,8 +431,8 @@ class TestGeneralAggregate(PostgreSQLTestCase):
         )
         self.assertEqual(values, {"stringagg": "Text1;Text2;Text4;Text3"})
 
-    def test_string_agg_charfield_ordering(self):
+    def test_string_agg_charfield_order_by(self):
-        ordering_test_cases = (
+        order_by_test_cases = (
             (F("char_field").desc(), "Foo4;Foo3;Foo2;Foo1"),
             (F("char_field").asc(), "Foo1;Foo2;Foo3;Foo4"),
             (F("char_field"), "Foo1;Foo2;Foo3;Foo4"),
@@ -416,19 +441,19 @@ class TestGeneralAggregate(PostgreSQLTestCase):
             (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:
+        for order_by, expected_output in order_by_test_cases:
-            with self.subTest(ordering=ordering, expected_output=expected_output):
+            with self.subTest(order_by=order_by, expected_output=expected_output):
                 values = AggregateTestModel.objects.aggregate(
-                    stringagg=StringAgg("char_field", delimiter=";", ordering=ordering)
+                    stringagg=StringAgg("char_field", delimiter=";", order_by=order_by)
                 )
                 self.assertEqual(values, {"stringagg": expected_output})
 
-    def test_string_agg_jsonfield_ordering(self):
+    def test_string_agg_jsonfield_order_by(self):
         values = AggregateTestModel.objects.aggregate(
             stringagg=StringAgg(
                 KeyTextTransform("lang", "json_field"),
                 delimiter=";",
-                ordering=KeyTextTransform("lang", "json_field"),
+                order_by=KeyTextTransform("lang", "json_field"),
                 output_field=CharField(),
             ),
         )
@@ -446,7 +471,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
 
     def test_orderable_agg_alternative_fields(self):
         values = AggregateTestModel.objects.aggregate(
-            arrayagg=ArrayAgg("integer_field", ordering=F("char_field").asc())
+            arrayagg=ArrayAgg("integer_field", order_by=F("char_field").asc())
         )
         self.assertEqual(values, {"arrayagg": [0, 1, 0, 2]})
 
@@ -454,8 +479,8 @@ class TestGeneralAggregate(PostgreSQLTestCase):
         values = AggregateTestModel.objects.aggregate(jsonbagg=JSONBAgg("char_field"))
         self.assertEqual(values, {"jsonbagg": ["Foo1", "Foo2", "Foo4", "Foo3"]})
 
-    def test_jsonb_agg_charfield_ordering(self):
+    def test_jsonb_agg_charfield_order_by(self):
-        ordering_test_cases = (
+        order_by_test_cases = (
             (F("char_field").desc(), ["Foo4", "Foo3", "Foo2", "Foo1"]),
             (F("char_field").asc(), ["Foo1", "Foo2", "Foo3", "Foo4"]),
             (F("char_field"), ["Foo1", "Foo2", "Foo3", "Foo4"]),
@@ -464,38 +489,38 @@ class TestGeneralAggregate(PostgreSQLTestCase):
             (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:
+        for order_by, expected_output in order_by_test_cases:
-            with self.subTest(ordering=ordering, expected_output=expected_output):
+            with self.subTest(order_by=order_by, expected_output=expected_output):
                 values = AggregateTestModel.objects.aggregate(
-                    jsonbagg=JSONBAgg("char_field", ordering=ordering),
+                    jsonbagg=JSONBAgg("char_field", order_by=order_by),
                 )
                 self.assertEqual(values, {"jsonbagg": expected_output})
 
-    def test_jsonb_agg_integerfield_ordering(self):
+    def test_jsonb_agg_integerfield_order_by(self):
         values = AggregateTestModel.objects.aggregate(
-            jsonbagg=JSONBAgg("integer_field", ordering=F("integer_field").desc()),
+            jsonbagg=JSONBAgg("integer_field", order_by=F("integer_field").desc()),
         )
         self.assertEqual(values, {"jsonbagg": [2, 1, 0, 0]})
 
-    def test_jsonb_agg_booleanfield_ordering(self):
+    def test_jsonb_agg_booleanfield_order_by(self):
-        ordering_test_cases = (
+        order_by_test_cases = (
             (F("boolean_field").asc(), [False, False, True, True]),
             (F("boolean_field").desc(), [True, True, False, False]),
             (F("boolean_field"), [False, False, True, True]),
         )
-        for ordering, expected_output in ordering_test_cases:
+        for order_by, expected_output in order_by_test_cases:
-            with self.subTest(ordering=ordering, expected_output=expected_output):
+            with self.subTest(order_by=order_by, expected_output=expected_output):
                 values = AggregateTestModel.objects.aggregate(
-                    jsonbagg=JSONBAgg("boolean_field", ordering=ordering),
+                    jsonbagg=JSONBAgg("boolean_field", order_by=order_by),
                 )
                 self.assertEqual(values, {"jsonbagg": expected_output})
 
-    def test_jsonb_agg_jsonfield_ordering(self):
+    def test_jsonb_agg_jsonfield_order_by(self):
         values = AggregateTestModel.objects.aggregate(
             jsonbagg=JSONBAgg(
                 KeyTransform("lang", "json_field"),
                 filter=Q(json_field__lang__isnull=False),
-                ordering=KeyTransform("lang", "json_field"),
+                order_by=KeyTransform("lang", "json_field"),
             ),
         )
         self.assertEqual(values, {"jsonbagg": ["en", "pl"]})
@@ -533,7 +558,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
             Room.objects.annotate(
                 requirements=JSONBAgg(
                     "hotelreservation__requirements",
-                    ordering="-hotelreservation__start",
+                    order_by="-hotelreservation__start",
                 )
             )
             .filter(requirements__0__sea_view=True)
@@ -552,7 +577,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
             ],
         )
 
-    def test_string_agg_array_agg_ordering_in_subquery(self):
+    def test_string_agg_array_agg_order_by_in_subquery(self):
         stats = []
         for i, agg in enumerate(AggregateTestModel.objects.order_by("char_field")):
             stats.append(StatTestModel(related_field=agg, int1=i, int2=i + 1))
@@ -561,7 +586,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
 
         for aggregate, expected_result in (
             (
-                ArrayAgg("stattestmodel__int1", ordering="-stattestmodel__int2"),
+                ArrayAgg("stattestmodel__int1", order_by="-stattestmodel__int2"),
                 [
                     ("Foo1", [0, 1]),
                     ("Foo2", [1, 2]),
@@ -573,7 +598,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
                 StringAgg(
                     Cast("stattestmodel__int1", CharField()),
                     delimiter=";",
-                    ordering="-stattestmodel__int2",
+                    order_by="-stattestmodel__int2",
                 ),
                 [("Foo1", "0;1"), ("Foo2", "1;2"), ("Foo3", "2;3"), ("Foo4", "3;4")],
             ),