From 98ea4f0f4696221f00e111f1d623452002192e6c Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sun, 5 Apr 2020 15:32:54 -0400 Subject: [PATCH] Refs #7098 -- Deprecated passing raw column aliases to order_by(). Now that order_by() has expression support passing RawSQL() can achieve the same result. This was also already supported through QuerySet.extra(order_by) for years but this API is more or less deprecated at this point. --- django/db/models/sql/query.py | 11 ++++++++++- docs/internals/deprecation.txt | 3 +++ docs/releases/3.1.txt | 4 ++++ tests/queries/tests.py | 30 +++++++++++++++++++++++++++--- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index e5524a8198..82ff6bf4ea 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1895,7 +1895,16 @@ class Query(BaseExpression): """ errors = [] for item in ordering: - if not hasattr(item, 'resolve_expression') and not ORDER_PATTERN.match(item): + if isinstance(item, str) and ORDER_PATTERN.match(item): + if '.' in item: + warnings.warn( + 'Passing column raw column aliases to order_by() is ' + 'deprecated. Wrap %r in a RawSQL expression before ' + 'passing it to order_by().' % item, + category=RemovedInDjango40Warning, + stacklevel=3, + ) + elif not hasattr(item, 'resolve_expression'): errors.append(item) if getattr(item, 'contains_aggregate', False): raise FieldError( diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 7fea512750..257b9a3bf4 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -69,6 +69,9 @@ details on these changes. * ``django.views.generic.TemplateView`` will no longer pass URL kwargs directly to the ``context``. +* Support for passing raw column aliases to ``QuerySet.order_by()`` will be + removed. + See the :ref:`Django 3.1 release notes ` for more details on these changes. diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 70241b1809..45cd858f0f 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -678,6 +678,10 @@ Miscellaneous :class:`~django.views.generic.base.TemplateView` is deprecated. Reference them in the template with ``view.kwargs`` instead. +* Passing raw column aliases to :meth:`.QuerySet.order_by` is deprecated. The + same result can be achieved by passing aliases in a + :class:`~django.db.models.expressions.RawSQL` instead beforehand. + .. _removed-features-3.1: Features removed in 3.1 diff --git a/tests/queries/tests.py b/tests/queries/tests.py index e57cad79ef..532aa07270 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -7,10 +7,12 @@ from operator import attrgetter from django.core.exceptions import EmptyResultSet, FieldError from django.db import DEFAULT_DB_ALIAS, connection from django.db.models import Count, Exists, F, OuterRef, Q +from django.db.models.expressions import RawSQL from django.db.models.sql.constants import LOUTER from django.db.models.sql.where import NothingNode, WhereNode from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature -from django.test.utils import CaptureQueriesContext +from django.test.utils import CaptureQueriesContext, ignore_warnings +from django.utils.deprecation import RemovedInDjango40Warning from .models import ( FK1, Annotation, Article, Author, BaseA, Book, CategoryItem, @@ -609,14 +611,36 @@ class Queries1Tests(TestCase): ['datetime.datetime(2007, 12, 19, 0, 0)'] ) + @ignore_warnings(category=RemovedInDjango40Warning) def test_ticket7098(self): - # Make sure semi-deprecated ordering by related models syntax still - # works. self.assertSequenceEqual( Item.objects.values('note__note').order_by('queries_note.note', 'id'), [{'note__note': 'n2'}, {'note__note': 'n3'}, {'note__note': 'n3'}, {'note__note': 'n3'}] ) + def test_order_by_rawsql(self): + self.assertSequenceEqual( + Item.objects.values('note__note').order_by( + RawSQL('queries_note.note', ()), + 'id', + ), + [ + {'note__note': 'n2'}, + {'note__note': 'n3'}, + {'note__note': 'n3'}, + {'note__note': 'n3'}, + ], + ) + + def test_order_by_raw_column_alias_warning(self): + msg = ( + "Passing column raw column aliases to order_by() is deprecated. " + "Wrap 'queries_author.name' in a RawSQL expression before " + "passing it to order_by()." + ) + with self.assertRaisesMessage(RemovedInDjango40Warning, msg): + Item.objects.values('creator__name').order_by('queries_author.name') + def test_ticket7096(self): # Make sure exclude() with multiple conditions continues to work. self.assertQuerysetEqual(