From 0be8095b254fad65b2480d677ebe6098c41bbad6 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Fri, 6 Jan 2023 13:53:42 +0100 Subject: [PATCH] Refs #10929 -- Stopped forcing empty result value by PostgreSQL aggregates. Per deprecation timeline. --- django/contrib/postgres/aggregates/general.py | 67 ++-------------- docs/ref/contrib/postgres/aggregates.txt | 27 +++---- docs/releases/5.0.txt | 4 + tests/postgres_tests/test_aggregates.py | 78 ++++--------------- 4 files changed, 34 insertions(+), 142 deletions(-) diff --git a/django/contrib/postgres/aggregates/general.py b/django/contrib/postgres/aggregates/general.py index 3de59dfcfd..c8eea1034d 100644 --- a/django/contrib/postgres/aggregates/general.py +++ b/django/contrib/postgres/aggregates/general.py @@ -3,7 +3,7 @@ import warnings from django.contrib.postgres.fields import ArrayField from django.db.models import Aggregate, BooleanField, JSONField, TextField, Value -from django.utils.deprecation import RemovedInDjango50Warning, RemovedInDjango51Warning +from django.utils.deprecation import RemovedInDjango51Warning from .mixins import OrderableAggMixin @@ -19,47 +19,11 @@ __all__ = [ ] -# 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 resolve_expression(self, *args, **kwargs): - resolved = super().resolve_expression(*args, **kwargs) - if not self._default_provided: - resolved.empty_result_set_value = getattr( - self, "deprecation_empty_result_set_value", self.deprecation_value - ) - return resolved - - 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): +class ArrayAgg(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=[] to keep the previous behavior." - ) - @property def output_field(self): return ArrayField(self.source_expressions[0].output_field) @@ -87,27 +51,14 @@ class BoolOr(Aggregate): output_field = BooleanField() -class JSONBAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate): +class JSONBAgg(OrderableAggMixin, Aggregate): function = "JSONB_AGG" template = "%(function)s(%(distinct)s%(expressions)s %(ordering)s)" allow_distinct = True output_field = JSONField() - # RemovedInDjango50Warning - deprecation_value = "[]" - deprecation_empty_result_set_value = property(lambda self: []) - 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=[] to keep the previous " - "behavior." - ) - # RemovedInDjango51Warning: When the deprecation ends, remove __init__(). - # - # RemovedInDjango50Warning: When the deprecation ends, replace with: - # def __init__(self, *expressions, default=None, **extra): - def __init__(self, *expressions, default=NOT_PROVIDED, **extra): + def __init__(self, *expressions, default=None, **extra): super().__init__(*expressions, default=default, **extra) if ( isinstance(default, Value) @@ -136,20 +87,12 @@ class JSONBAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate): ) -class StringAgg(DeprecatedConvertValueMixin, OrderableAggMixin, Aggregate): +class StringAgg(OrderableAggMixin, Aggregate): function = "STRING_AGG" template = "%(function)s(%(distinct)s%(expressions)s %(ordering)s)" allow_distinct = True output_field = TextField() - # 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="" to keep the previous behavior.' - ) - def __init__(self, expression, delimiter, **extra): delimiter_expr = Value(str(delimiter)) super().__init__(expression, delimiter_expr, **extra) diff --git a/docs/ref/contrib/postgres/aggregates.txt b/docs/ref/contrib/postgres/aggregates.txt index 79cfa8432b..d304a75ce1 100644 --- a/docs/ref/contrib/postgres/aggregates.txt +++ b/docs/ref/contrib/postgres/aggregates.txt @@ -52,12 +52,11 @@ General-purpose aggregation functions from django.db.models import F F('some_field').desc() - .. deprecated:: 4.0 + .. versionchanged:: 5.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([])``. + In older versions, if there are no rows and ``default`` is not + provided, ``ArrayAgg`` returned an empty list instead of ``None``. If + you need it, explicitly set ``default`` to ``Value([])``. ``BitAnd`` ---------- @@ -173,12 +172,11 @@ General-purpose aggregation functions {'parking': True, 'double_bed': True} ]}]> - .. deprecated:: 4.0 + .. versionchanged:: 5.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('[]')``. + In older versions, if there are no rows and ``default`` is not + provided, ``JSONBAgg`` returned an empty list instead of ``None``. If + you need it, explicitly set ``default`` to ``Value([])``. ``StringAgg`` ------------- @@ -232,12 +230,11 @@ General-purpose aggregation functions 'headline': 'NASA uses Python', 'publication_names': 'Science News, The Python Journal' }]> - .. deprecated:: 4.0 + .. versionchanged:: 5.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('')``. + In older versions, if there are no rows and ``default`` is not + provided, ``StringAgg`` returned an empty string instead of ``None``. + If you need it, explicitly set ``default`` to ``Value("")``. Aggregate functions for statistics ================================== diff --git a/docs/releases/5.0.txt b/docs/releases/5.0.txt index 12478e0b89..011d01bec0 100644 --- a/docs/releases/5.0.txt +++ b/docs/releases/5.0.txt @@ -270,6 +270,10 @@ to remove usage of these features. * The ``extra_tests`` argument for ``DiscoverRunner.build_suite()`` and ``DiscoverRunner.run_tests()`` is removed. +* The ``django.contrib.postgres.aggregates.ArrayAgg``, ``JSONBAgg``, and + ``StringAgg`` aggregates no longer return ``[]``, ``[]``, and ``''``, + respectively, when there are no rows. + See :ref:`deprecated-features-4.1` for details on these changes, including how to remove usage of these features. diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py index b5474d361e..04839952f3 100644 --- a/tests/postgres_tests/test_aggregates.py +++ b/tests/postgres_tests/test_aggregates.py @@ -16,7 +16,7 @@ from django.db.models.functions import Cast, Concat, Substr from django.test import skipUnlessDBFeature from django.test.utils import Approximate, ignore_warnings from django.utils import timezone -from django.utils.deprecation import RemovedInDjango50Warning, RemovedInDjango51Warning +from django.utils.deprecation import RemovedInDjango51Warning from . import PostgreSQLTestCase from .models import AggregateTestModel, HotelReservation, Room, StatTestModel @@ -84,36 +84,35 @@ class TestGeneralAggregate(PostgreSQLTestCase): ] ) - @ignore_warnings(category=RemovedInDjango50Warning) def test_empty_result_set(self): AggregateTestModel.objects.all().delete() tests = [ - (ArrayAgg("char_field"), []), - (ArrayAgg("integer_field"), []), - (ArrayAgg("boolean_field"), []), - (BitAnd("integer_field"), None), - (BitOr("integer_field"), None), - (BoolAnd("boolean_field"), None), - (BoolOr("boolean_field"), None), - (JSONBAgg("integer_field"), []), - (StringAgg("char_field", delimiter=";"), ""), + ArrayAgg("char_field"), + ArrayAgg("integer_field"), + ArrayAgg("boolean_field"), + BitAnd("integer_field"), + BitOr("integer_field"), + BoolAnd("boolean_field"), + BoolOr("boolean_field"), + JSONBAgg("integer_field"), + StringAgg("char_field", delimiter=";"), ] if connection.features.has_bit_xor: tests.append((BitXor("integer_field"), None)) - for aggregation, expected_result in tests: + for aggregation in tests: with self.subTest(aggregation=aggregation): # Empty result with non-execution optimization. with self.assertNumQueries(0): values = AggregateTestModel.objects.none().aggregate( aggregation=aggregation, ) - self.assertEqual(values, {"aggregation": expected_result}) + self.assertEqual(values, {"aggregation": None}) # Empty result when query must be executed. with self.assertNumQueries(1): values = AggregateTestModel.objects.aggregate( aggregation=aggregation, ) - self.assertEqual(values, {"aggregation": expected_result}) + self.assertEqual(values, {"aggregation": None}) def test_default_argument(self): AggregateTestModel.objects.all().delete() @@ -153,57 +152,6 @@ 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=[])), - {"aggregation": []}, - ) - self.assertEqual( - queryset.aggregate( - aggregation=StringAgg("char_field", delimiter=";", default=Value("")), - ), - {"aggregation": ""}, - ) - @ignore_warnings(category=RemovedInDjango51Warning) def test_jsonb_agg_default_str_value(self): AggregateTestModel.objects.all().delete()