From 0ddb4ebf7bfcc4730c80a772dd146a49ef6895f6 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Sat, 7 Sep 2019 19:28:19 +0200 Subject: [PATCH] Refs #14357 -- Made Meta.ordering not affect GROUP BY queries. Per deprecation timeline. --- django/db/models/sql/compiler.py | 14 +------------- docs/ref/models/options.txt | 4 ---- docs/releases/3.1.txt | 2 ++ docs/topics/db/aggregation.txt | 7 ------- tests/aggregation_regress/tests.py | 15 ++------------- tests/ordering/tests.py | 13 +------------ tests/queries/test_explain.py | 6 +----- 7 files changed, 7 insertions(+), 54 deletions(-) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 5da862f716..e287a6f55a 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1,6 +1,5 @@ import collections import re -import warnings from itertools import chain from django.core.exceptions import EmptyResultSet, FieldError @@ -14,7 +13,6 @@ from django.db.models.sql.constants import ( from django.db.models.sql.query import Query, get_order_dir from django.db.transaction import TransactionManagementError from django.db.utils import DatabaseError, NotSupportedError -from django.utils.deprecation import RemovedInDjango31Warning from django.utils.hashable import make_hashable @@ -565,17 +563,7 @@ class SQLCompiler: order_by = order_by or self.connection.ops.force_no_ordering() result.append('GROUP BY %s' % ', '.join(grouping)) if self._meta_ordering: - # When the deprecation ends, replace with: - # order_by = None - warnings.warn( - "%s QuerySet won't use Meta.ordering in Django 3.1. " - "Add .order_by(%s) to retain the current query." % ( - self.query.model.__name__, - ', '.join(repr(f) for f in self._meta_ordering), - ), - RemovedInDjango31Warning, - stacklevel=4, - ) + order_by = None if having: result.append('HAVING %s' % having) params.extend(h_params) diff --git a/docs/ref/models/options.txt b/docs/ref/models/options.txt index 87f1e4b15c..e1d927b4e5 100644 --- a/docs/ref/models/options.txt +++ b/docs/ref/models/options.txt @@ -284,10 +284,6 @@ Django quotes column and table names behind the scenes. ordering = [F('author').asc(nulls_last=True)] - Default ordering also affects :ref:`aggregation queries - ` but this won't be the case starting - in Django 3.1. - .. warning:: Ordering is not a free operation. Each field you add to the ordering diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 5f6af788ce..9b16bae9ca 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -234,6 +234,8 @@ to remove usage of these features. * ``django.core.paginator.QuerySetPaginator`` is removed. +* A model's ``Meta.ordering`` doesn't affect ``GROUP BY`` queries. + * ``django.contrib.postgres.fields.FloatRangeField`` and ``django.contrib.postgres.forms.FloatRangeField`` are removed. diff --git a/docs/topics/db/aggregation.txt b/docs/topics/db/aggregation.txt index 6e5c6271dd..0a64c703dc 100644 --- a/docs/topics/db/aggregation.txt +++ b/docs/topics/db/aggregation.txt @@ -512,13 +512,6 @@ include the aggregate column. Interaction with default ordering or ``order_by()`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -.. deprecated:: 2.2 - - Starting in Django 3.1, the ordering from a model's ``Meta.ordering`` won't - be used in ``GROUP BY`` queries, such as ``.annotate().values()``. Since - Django 2.2, these queries issue a deprecation warning indicating to add an - explicit ``order_by()`` to the queryset to silence the warning. - Fields that are mentioned in the ``order_by()`` part of a queryset (or which are used in the default ordering on a model) are used when selecting the output data, even if they are not otherwise specified in the ``values()`` diff --git a/tests/aggregation_regress/tests.py b/tests/aggregation_regress/tests.py index 6234d3c590..63e51fea2e 100644 --- a/tests/aggregation_regress/tests.py +++ b/tests/aggregation_regress/tests.py @@ -12,11 +12,8 @@ from django.db.models import ( Value, Variance, When, ) from django.db.models.aggregates import Aggregate -from django.test import ( - TestCase, ignore_warnings, skipUnlessAnyDBFeature, skipUnlessDBFeature, -) +from django.test import TestCase, skipUnlessAnyDBFeature, skipUnlessDBFeature from django.test.utils import Approximate -from django.utils.deprecation import RemovedInDjango31Warning from .models import ( Alfa, Author, Book, Bravo, Charlie, Clues, Entries, HardbackBook, ItemTag, @@ -110,7 +107,6 @@ class AggregationTests(TestCase): for attr, value in kwargs.items(): self.assertEqual(getattr(obj, attr), value) - @ignore_warnings(category=RemovedInDjango31Warning) def test_annotation_with_value(self): values = Book.objects.filter( name='Practical Django Projects', @@ -218,7 +214,6 @@ class AggregationTests(TestCase): {'pages__sum': 3703} ) - @ignore_warnings(category=RemovedInDjango31Warning) def test_annotation(self): # Annotations get combined with extra select clauses obj = Book.objects.annotate(mean_auth_age=Avg("authors__age")).extra( @@ -312,8 +307,7 @@ class AggregationTests(TestCase): # If an annotation isn't included in the values, it can still be used # in a filter - with ignore_warnings(category=RemovedInDjango31Warning): - qs = Book.objects.annotate(n_authors=Count('authors')).values('name').filter(n_authors__gt=2) + qs = Book.objects.annotate(n_authors=Count('authors')).values('name').filter(n_authors__gt=2) self.assertSequenceEqual( qs, [ {"name": 'Python Web Development with Django'} @@ -457,7 +451,6 @@ class AggregationTests(TestCase): with self.assertRaisesMessage(FieldError, msg): Book.objects.all().annotate(num_authors=Count('authors__id')).aggregate(Max('foo')) - @ignore_warnings(category=RemovedInDjango31Warning) def test_more(self): # Old-style count aggregations can be mixed with new-style self.assertEqual( @@ -811,7 +804,6 @@ class AggregationTests(TestCase): with self.assertRaisesMessage(ValueError, msg): Author.objects.annotate(book_contact_set=Avg('friends__age')) - @ignore_warnings(category=RemovedInDjango31Warning) def test_pickle(self): # Regression for #10197 -- Queries with aggregates can be pickled. # First check that pickling is possible at all. No crash = success @@ -1210,7 +1202,6 @@ class AggregationTests(TestCase): {'book__count__max': 2} ) - @ignore_warnings(category=RemovedInDjango31Warning) def test_annotate_joins(self): """ The base table's join isn't promoted to LOUTER. This could @@ -1474,7 +1465,6 @@ class AggregationTests(TestCase): query.filter(q1 | q2) self.assertEqual(len(q2.children), 1) - @ignore_warnings(category=RemovedInDjango31Warning) def test_fobj_group_by(self): """ An F() object referring to related column works correctly in group by. @@ -1562,7 +1552,6 @@ class JoinPromotionTests(TestCase): qs = Charlie.objects.annotate(Count('alfa__name')) self.assertIn(' LEFT OUTER JOIN ', str(qs.query)) - @ignore_warnings(category=RemovedInDjango31Warning) def test_non_nullable_fk_not_promoted(self): qs = Book.objects.annotate(Count('contact__name')) self.assertIn(' INNER JOIN ', str(qs.query)) diff --git a/tests/ordering/tests.py b/tests/ordering/tests.py index 5ee3c60324..a9cc729c45 100644 --- a/tests/ordering/tests.py +++ b/tests/ordering/tests.py @@ -3,11 +3,10 @@ from operator import attrgetter from django.core.exceptions import FieldError from django.db.models import ( - CharField, Count, DateTimeField, F, Max, OuterRef, Subquery, Value, + CharField, DateTimeField, F, Max, OuterRef, Subquery, Value, ) from django.db.models.functions import Upper from django.test import TestCase -from django.utils.deprecation import RemovedInDjango31Warning from .models import Article, Author, ChildArticle, OrderedByFArticle, Reference @@ -481,13 +480,3 @@ class OrderingTests(TestCase): ca4 = ChildArticle.objects.create(headline='h1', pub_date=datetime(2005, 7, 28)) articles = ChildArticle.objects.order_by('article_ptr') self.assertSequenceEqual(articles, [ca4, ca2, ca1, ca3]) - - def test_deprecated_values_annotate(self): - msg = ( - "Article QuerySet won't use Meta.ordering in Django 3.1. Add " - ".order_by('-pub_date', F(headline), OrderBy(F(author__name), " - "descending=False), OrderBy(F(second_author__name), " - "descending=False)) to retain the current query." - ) - with self.assertRaisesMessage(RemovedInDjango31Warning, msg): - list(Article.objects.values('author').annotate(Count('headline'))) diff --git a/tests/queries/test_explain.py b/tests/queries/test_explain.py index 9428bd88e9..ad4ca988ee 100644 --- a/tests/queries/test_explain.py +++ b/tests/queries/test_explain.py @@ -2,11 +2,8 @@ import unittest from django.db import NotSupportedError, connection, transaction from django.db.models import Count -from django.test import ( - TestCase, ignore_warnings, skipIfDBFeature, skipUnlessDBFeature, -) +from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature from django.test.utils import CaptureQueriesContext -from django.utils.deprecation import RemovedInDjango31Warning from .models import Tag @@ -14,7 +11,6 @@ from .models import Tag @skipUnlessDBFeature('supports_explaining_query_execution') class ExplainTests(TestCase): - @ignore_warnings(category=RemovedInDjango31Warning) def test_basic(self): querysets = [ Tag.objects.filter(name='test'),