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

camuthig · sarahboyce · commit d734f1651ccc · 2025-01-06T09:39:21.000+01:00
Aligns the argument with SQL standards already used in Window.order_by and
sets up for adding support to Aggregate.
diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py
@@ -17,7 +17,7 @@
 
 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()
 
diff --git a/django/contrib/postgres/aggregates/mixins.py b/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):
-        if not ordering:
+    # RemovedInDjango61Warning: When the deprecation ends, replace with:
+    # 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)):
-            self.order_by = OrderByList(*ordering)
+        elif isinstance(order_by, (list, tuple)):
+            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 @@ def set_source_expressions(self, exprs):
         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:
-            order_by_sql, order_by_params = compiler.compile(ordering_expr)
+        if order_by_expr is not None:
+            order_by_sql, order_by_params = compiler.compile(order_by_expr)
 
         filter_params = []
         if filtering_expr is not None:
@@ -43,5 +58,5 @@ def as_sql(self, compiler, connection):
         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)
diff --git a/docs/internals/deprecation.txt b/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
diff --git a/docs/ref/contrib/postgres/aggregates.txt b/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,18 +177,23 @@ 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': [
             {'parking': False, 'sea_view': True, 'double_bed': False},
             {'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
 ==================================
 
diff --git a/docs/releases/3.2.txt b/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
diff --git a/docs/releases/5.2.txt b/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.
diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py
