summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Pope <nick@nickpope.me.uk>2021-06-30 00:08:27 +0100
committerMariusz Felisiak <felisiak.mariusz@gmail.com>2021-07-19 13:41:16 +0200
commitfee87345967b3d917b618533585076cbfa43451b (patch)
tree202561eec2284ce1bd3ae4793d32ac346b068848
parent501a8db46595b2d5b99c1d3b1146a832f43cdf1c (diff)
Refs #10929 -- Deprecated forced empty result value for PostgreSQL aggregates.
This deprecates forcing a return value for ArrayAgg, JSONBAgg, and StringAgg when there are no rows in the query. Now that we have a ``default`` argument for aggregates, we want to revert to returning the default of ``None`` which most aggregate functions return and leave it up to the user to decide what they want to be returned by default.
-rw-r--r--django/contrib/postgres/aggregates/general.py69
-rw-r--r--docs/internals/deprecation.txt4
-rw-r--r--docs/ref/contrib/postgres/aggregates.txt21
-rw-r--r--docs/releases/4.0.txt7
-rw-r--r--tests/postgres_tests/test_aggregates.py47
5 files changed, 130 insertions, 18 deletions
diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py
index d36f71fddf..ae432a016c 100644
--- a/django/contrib/postgres/aggregates/general.py
+++ b/django/contrib/postgres/aggregates/general.py
@@ -1,5 +1,8 @@
+import warnings
+
from django.contrib.postgres.fields import ArrayField
from django.db.models import Aggregate, BooleanField, JSONField, Value
+from django.utils.deprecation import RemovedInDjango50Warning
from .mixins import OrderableAggMixin
@@ -8,20 +11,44 @@ __all__ = [
]
-class ArrayAgg(OrderableAggMixin, Aggregate):
+# RemovedInDjango50Warning
+NOT_PROVIDED = object()
+
+
+class DeprecatedConvertValueMixin:
+ def __init__(self, *expressions, default=NOT_PROVIDED, **extra):
+ if default is NOT_PROVIDED:
+ default = None
+ self._default_provided = False
+ else:
+ self._default_provided = True
+ super().__init__(*expressions, default=default, **extra)
+
+ def convert_value(self, value, expression, connection):
+ if value is None and not self._default_provided:
+ warnings.warn(self.deprecation_msg, category=RemovedInDjango50Warning)
+ return self.deprecation_value
+ return value
+
+
+class ArrayAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
function = 'ARRAY_AGG'
template = '%(function)s(%(distinct)s%(expressions)s %(ordering)s)'
allow_distinct = True
+ # RemovedInDjango50Warning
+ deprecation_value = property(lambda self: [])
+ deprecation_msg = (
+ 'In Django 5.0, ArrayAgg() will return None instead of an empty list '
+ 'if there are no rows. Pass default=None to opt into the new behavior '
+ 'and silence this warning or default=Value([]) to keep the previous '
+ 'behavior.'
+ )
+
@property
def output_field(self):
return ArrayField(self.source_expressions[0].output_field)
- def convert_value(self, value, expression, connection):
- if value is None and self.default is None:
- return []
- return value
-
class BitAnd(Aggregate):
function = 'BIT_AND'
@@ -41,28 +68,36 @@ class BoolOr(Aggregate):
output_field = BooleanField()
-class JSONBAgg(OrderableAggMixin, Aggregate):
+class JSONBAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
function = 'JSONB_AGG'
template = '%(function)s(%(distinct)s%(expressions)s %(ordering)s)'
allow_distinct = True
output_field = JSONField()
- def convert_value(self, value, expression, connection):
- if value is None and self.default is None:
- return '[]'
- return value
+ # RemovedInDjango50Warning
+ deprecation_value = '[]'
+ deprecation_msg = (
+ "In Django 5.0, JSONBAgg() will return None instead of an empty list "
+ "if there are no rows. Pass default=None to opt into the new behavior "
+ "and silence this warning or default=Value('[]') to keep the previous "
+ "behavior."
+ )
-class StringAgg(OrderableAggMixin, Aggregate):
+class StringAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate):
function = 'STRING_AGG'
template = '%(function)s(%(distinct)s%(expressions)s %(ordering)s)'
allow_distinct = True
+ # RemovedInDjango50Warning
+ deprecation_value = ''
+ deprecation_msg = (
+ "In Django 5.0, StringAgg() will return None instead of an empty "
+ "string if there are no rows. Pass default=None to opt into the new "
+ "behavior and silence this warning or default=Value('') to keep the "
+ "previous behavior."
+ )
+
def __init__(self, expression, delimiter, **extra):
delimiter_expr = Value(str(delimiter))
super().__init__(expression, delimiter_expr, **extra)
-
- def convert_value(self, value, expression, connection):
- if value is None and self.default is None:
- return ''
- return value
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index c56b875aed..16801e0345 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -30,6 +30,10 @@ details on these changes.
* The ``extra_tests`` argument for ``DiscoverRunner.build_suite()`` and
``DiscoverRunner.run_tests()`` will be removed.
+* The ``django.contrib.postgres.aggregates.ArrayAgg``, ``JSONBAgg``, and
+ ``StringAgg`` aggregates will return ``None`` when there are no rows instead
+ of ``[]``, ``[]``, and ``''`` respectively.
+
.. _deprecation-removed-in-4.1:
4.1
diff --git a/docs/ref/contrib/postgres/aggregates.txt b/docs/ref/contrib/postgres/aggregates.txt
index 61ec86fa44..6d85d5c478 100644
--- a/docs/ref/contrib/postgres/aggregates.txt
+++ b/docs/ref/contrib/postgres/aggregates.txt
@@ -52,6 +52,13 @@ General-purpose aggregation functions
from django.db.models import F
F('some_field').desc()
+ .. deprecated:: 4.0
+
+ If there are no rows and ``default`` is not provided, ``ArrayAgg``
+ returns an empty list instead of ``None``. This behavior is deprecated
+ and will be removed in Django 5.0. If you need it, explicitly set
+ ``default`` to ``Value([])``.
+
``BitAnd``
----------
@@ -138,6 +145,13 @@ General-purpose aggregation functions
Examples are the same as for :attr:`ArrayAgg.ordering`.
+ .. deprecated:: 4.0
+
+ If there are no rows and ``default`` is not provided, ``JSONBAgg``
+ returns an empty list instead of ``None``. This behavior is deprecated
+ and will be removed in Django 5.0. If you need it, explicitly set
+ ``default`` to ``Value('[]')``.
+
``StringAgg``
-------------
@@ -164,6 +178,13 @@ General-purpose aggregation functions
Examples are the same as for :attr:`ArrayAgg.ordering`.
+ .. deprecated:: 4.0
+
+ If there are no rows and ``default`` is not provided, ``StringAgg``
+ returns an empty string instead of ``None``. This behavior is
+ deprecated and will be removed in Django 5.0. If you need it,
+ explicitly set ``default`` to ``Value('')``.
+
Aggregate functions for statistics
==================================
diff --git a/docs/releases/4.0.txt b/docs/releases/4.0.txt
index 4122c9b419..244f113c54 100644
--- a/docs/releases/4.0.txt
+++ b/docs/releases/4.0.txt
@@ -540,6 +540,13 @@ Miscellaneous
* The ``extra_tests`` argument for :meth:`.DiscoverRunner.build_suite` and
:meth:`.DiscoverRunner.run_tests` is deprecated.
+* The :class:`~django.contrib.postgres.aggregates.ArrayAgg`,
+ :class:`~django.contrib.postgres.aggregates.JSONBAgg`, and
+ :class:`~django.contrib.postgres.aggregates.StringAgg` aggregates will return
+ ``None`` when there are no rows instead of ``[]``, ``[]``, and ``''``
+ respectively in Django 5.0. If you need the previous behavior, explicitly set
+ ``default`` to ``Value([])``, ``Value('[]')``, or ``Value('')``.
+
Features removed in 4.0
=======================
diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py
index 393e3d38e8..7ae7b16c9f 100644
--- a/tests/postgres_tests/test_aggregates.py
+++ b/tests/postgres_tests/test_aggregates.py
@@ -3,7 +3,8 @@ from django.db.models import (
)
from django.db.models.fields.json import KeyTextTransform, KeyTransform
from django.db.models.functions import Cast, Concat, Substr
-from django.test.utils import Approximate
+from django.test.utils import Approximate, ignore_warnings
+from django.utils.deprecation import RemovedInDjango50Warning
from . import PostgreSQLTestCase
from .models import AggregateTestModel, StatTestModel
@@ -44,6 +45,7 @@ class TestGeneralAggregate(PostgreSQLTestCase):
),
])
+ @ignore_warnings(category=RemovedInDjango50Warning)
def test_empty_result_set(self):
AggregateTestModel.objects.all().delete()
tests = [
@@ -100,6 +102,49 @@ class TestGeneralAggregate(PostgreSQLTestCase):
)
self.assertEqual(values, {'aggregation': expected_result})
+ def test_convert_value_deprecation(self):
+ AggregateTestModel.objects.all().delete()
+ queryset = AggregateTestModel.objects.all()
+
+ with self.assertWarnsMessage(RemovedInDjango50Warning, ArrayAgg.deprecation_msg):
+ queryset.aggregate(aggregation=ArrayAgg('boolean_field'))
+
+ with self.assertWarnsMessage(RemovedInDjango50Warning, JSONBAgg.deprecation_msg):
+ queryset.aggregate(aggregation=JSONBAgg('integer_field'))
+
+ with self.assertWarnsMessage(RemovedInDjango50Warning, StringAgg.deprecation_msg):
+ queryset.aggregate(aggregation=StringAgg('char_field', delimiter=';'))
+
+ # No warnings raised if default argument provided.
+ self.assertEqual(
+ queryset.aggregate(aggregation=ArrayAgg('boolean_field', default=None)),
+ {'aggregation': None},
+ )
+ self.assertEqual(
+ queryset.aggregate(aggregation=JSONBAgg('integer_field', default=None)),
+ {'aggregation': None},
+ )
+ self.assertEqual(
+ queryset.aggregate(
+ aggregation=StringAgg('char_field', delimiter=';', default=None),
+ ),
+ {'aggregation': None},
+ )
+ self.assertEqual(
+ queryset.aggregate(aggregation=ArrayAgg('boolean_field', default=Value([]))),
+ {'aggregation': []},
+ )
+ self.assertEqual(
+ queryset.aggregate(aggregation=JSONBAgg('integer_field', default=Value('[]'))),
+ {'aggregation': []},
+ )
+ self.assertEqual(
+ queryset.aggregate(
+ aggregation=StringAgg('char_field', delimiter=';', default=Value('')),
+ ),
+ {'aggregation': ''},
+ )
+
def test_array_agg_charfield(self):
values = AggregateTestModel.objects.aggregate(arrayagg=ArrayAgg('char_field'))
self.assertEqual(values, {'arrayagg': ['Foo1', 'Foo2', 'Foo4', 'Foo3']})