diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 148b19b462..0eb70b057d 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -2,6 +2,7 @@ import copy import datetime import functools import inspect +import warnings from collections import defaultdict from decimal import Decimal from uuid import UUID @@ -12,6 +13,7 @@ from django.db.models import fields from django.db.models.constants import LOOKUP_SEP from django.db.models.query_utils import Q from django.utils.deconstruct import deconstructible +from django.utils.deprecation import RemovedInDjango50Warning from django.utils.functional import cached_property from django.utils.hashable import make_hashable @@ -1513,11 +1515,20 @@ class OrderBy(Expression): template = "%(expression)s %(ordering)s" conditional = False - def __init__( - self, expression, descending=False, nulls_first=False, nulls_last=False - ): + def __init__(self, expression, descending=False, nulls_first=None, nulls_last=None): if nulls_first and nulls_last: raise ValueError("nulls_first and nulls_last are mutually exclusive") + if nulls_first is False or nulls_last is False: + # When the deprecation ends, replace with: + # raise ValueError( + # "nulls_first and nulls_last values must be True or None." + # ) + warnings.warn( + "Passing nulls_first=False or nulls_last=False is deprecated, use None " + "instead.", + RemovedInDjango50Warning, + stacklevel=2, + ) self.nulls_first = nulls_first self.nulls_last = nulls_last self.descending = descending @@ -1584,9 +1595,12 @@ class OrderBy(Expression): def reverse_ordering(self): self.descending = not self.descending - if self.nulls_first or self.nulls_last: - self.nulls_first = not self.nulls_first - self.nulls_last = not self.nulls_last + if self.nulls_first: + self.nulls_last = True + self.nulls_first = None + elif self.nulls_last: + self.nulls_first = True + self.nulls_last = None return self def asc(self): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 39007eb3e9..bb6b889f91 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -105,6 +105,10 @@ details on these changes. * The ``django.contrib.auth.hashers.CryptPasswordHasher`` will be removed. +* The ability to pass ``nulls_first=False`` or ``nulls_last=False`` to + ``Expression.asc()`` and ``Expression.desc()`` methods, and the ``OrderBy`` + expression will be removed. + .. _deprecation-removed-in-4.1: 4.1 diff --git a/docs/ref/models/expressions.txt b/docs/ref/models/expressions.txt index b3702116d7..da533ba5c3 100644 --- a/docs/ref/models/expressions.txt +++ b/docs/ref/models/expressions.txt @@ -1033,20 +1033,40 @@ calling the appropriate methods on the wrapped expression. to a column. The ``alias`` parameter will be ``None`` unless the expression has been annotated and is used for grouping. - .. method:: asc(nulls_first=False, nulls_last=False) + .. method:: asc(nulls_first=None, nulls_last=None) Returns the expression ready to be sorted in ascending order. ``nulls_first`` and ``nulls_last`` define how null values are sorted. See :ref:`using-f-to-sort-null-values` for example usage. - .. method:: desc(nulls_first=False, nulls_last=False) + .. versionchanged:: 4.1 + + In older versions, ``nulls_first`` and ``nulls_last`` defaulted to + ``False``. + + .. deprecated:: 4.1 + + Passing ``nulls_first=False`` or ``nulls_last=False`` to ``asc()`` + is deprecated. Use ``None`` instead. + + .. method:: desc(nulls_first=None, nulls_last=None) Returns the expression ready to be sorted in descending order. ``nulls_first`` and ``nulls_last`` define how null values are sorted. See :ref:`using-f-to-sort-null-values` for example usage. + .. versionchanged:: 4.1 + + In older versions, ``nulls_first`` and ``nulls_last`` defaulted to + ``False``. + + .. deprecated:: 4.1 + + Passing ``nulls_first=False`` or ``nulls_last=False`` to ``desc()`` + is deprecated. Use ``None`` instead. + .. method:: reverse_ordering() Returns ``self`` with any modifications required to reverse the sort diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index af129a149e..84eca03563 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -685,6 +685,10 @@ Miscellaneous * ``django.contrib.auth.hashers.CryptPasswordHasher`` is deprecated. +* The ability to pass ``nulls_first=False`` or ``nulls_last=False`` to + ``Expression.asc()`` and ``Expression.desc()`` methods, and the ``OrderBy`` + expression is deprecated. Use ``None`` instead. + Features removed in 4.1 ======================= diff --git a/tests/expressions/tests.py b/tests/expressions/tests.py index 72e6020fa0..39e6c18b1a 100644 --- a/tests/expressions/tests.py +++ b/tests/expressions/tests.py @@ -69,6 +69,7 @@ from django.test.utils import ( isolate_apps, register_lookup, ) +from django.utils.deprecation import RemovedInDjango50Warning from django.utils.functional import SimpleLazyObject from .models import ( @@ -2537,7 +2538,7 @@ class OrderByTests(SimpleTestCase): ) self.assertNotEqual( OrderBy(F("field"), nulls_last=True), - OrderBy(F("field"), nulls_last=False), + OrderBy(F("field")), ) def test_hash(self): @@ -2547,5 +2548,22 @@ class OrderByTests(SimpleTestCase): ) self.assertNotEqual( hash(OrderBy(F("field"), nulls_last=True)), - hash(OrderBy(F("field"), nulls_last=False)), + hash(OrderBy(F("field"))), ) + + def test_nulls_false(self): + # These tests will catch ValueError in Django 5.0 when passing False to + # nulls_first and nulls_last becomes forbidden. + # msg = "nulls_first and nulls_last values must be True or None." + msg = ( + "Passing nulls_first=False or nulls_last=False is deprecated, use None " + "instead." + ) + with self.assertRaisesMessage(RemovedInDjango50Warning, msg): + OrderBy(F("field"), nulls_first=False) + with self.assertRaisesMessage(RemovedInDjango50Warning, msg): + OrderBy(F("field"), nulls_last=False) + with self.assertRaisesMessage(RemovedInDjango50Warning, msg): + F("field").asc(nulls_first=False) + with self.assertRaisesMessage(RemovedInDjango50Warning, msg): + F("field").desc(nulls_last=False)