Refs #14357 -- Made Meta.ordering not affect GROUP BY queries.

Per deprecation timeline.
This commit is contained in:
Mariusz Felisiak 2019-09-07 19:28:19 +02:00
parent d17be88afd
commit 0ddb4ebf7b
7 changed files with 7 additions and 54 deletions

View File

@ -1,6 +1,5 @@
import collections import collections
import re import re
import warnings
from itertools import chain from itertools import chain
from django.core.exceptions import EmptyResultSet, FieldError 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.models.sql.query import Query, get_order_dir
from django.db.transaction import TransactionManagementError from django.db.transaction import TransactionManagementError
from django.db.utils import DatabaseError, NotSupportedError from django.db.utils import DatabaseError, NotSupportedError
from django.utils.deprecation import RemovedInDjango31Warning
from django.utils.hashable import make_hashable from django.utils.hashable import make_hashable
@ -565,17 +563,7 @@ class SQLCompiler:
order_by = order_by or self.connection.ops.force_no_ordering() order_by = order_by or self.connection.ops.force_no_ordering()
result.append('GROUP BY %s' % ', '.join(grouping)) result.append('GROUP BY %s' % ', '.join(grouping))
if self._meta_ordering: if self._meta_ordering:
# When the deprecation ends, replace with: order_by = None
# 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,
)
if having: if having:
result.append('HAVING %s' % having) result.append('HAVING %s' % having)
params.extend(h_params) params.extend(h_params)

View File

@ -284,10 +284,6 @@ Django quotes column and table names behind the scenes.
ordering = [F('author').asc(nulls_last=True)] ordering = [F('author').asc(nulls_last=True)]
Default ordering also affects :ref:`aggregation queries
<aggregation-ordering-interaction>` but this won't be the case starting
in Django 3.1.
.. warning:: .. warning::
Ordering is not a free operation. Each field you add to the ordering Ordering is not a free operation. Each field you add to the ordering

View File

@ -234,6 +234,8 @@ to remove usage of these features.
* ``django.core.paginator.QuerySetPaginator`` is removed. * ``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.fields.FloatRangeField`` and
``django.contrib.postgres.forms.FloatRangeField`` are removed. ``django.contrib.postgres.forms.FloatRangeField`` are removed.

View File

@ -512,13 +512,6 @@ include the aggregate column.
Interaction with default ordering or ``order_by()`` 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 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 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()`` output data, even if they are not otherwise specified in the ``values()``

View File

@ -12,11 +12,8 @@ from django.db.models import (
Value, Variance, When, Value, Variance, When,
) )
from django.db.models.aggregates import Aggregate from django.db.models.aggregates import Aggregate
from django.test import ( from django.test import TestCase, skipUnlessAnyDBFeature, skipUnlessDBFeature
TestCase, ignore_warnings, skipUnlessAnyDBFeature, skipUnlessDBFeature,
)
from django.test.utils import Approximate from django.test.utils import Approximate
from django.utils.deprecation import RemovedInDjango31Warning
from .models import ( from .models import (
Alfa, Author, Book, Bravo, Charlie, Clues, Entries, HardbackBook, ItemTag, Alfa, Author, Book, Bravo, Charlie, Clues, Entries, HardbackBook, ItemTag,
@ -110,7 +107,6 @@ class AggregationTests(TestCase):
for attr, value in kwargs.items(): for attr, value in kwargs.items():
self.assertEqual(getattr(obj, attr), value) self.assertEqual(getattr(obj, attr), value)
@ignore_warnings(category=RemovedInDjango31Warning)
def test_annotation_with_value(self): def test_annotation_with_value(self):
values = Book.objects.filter( values = Book.objects.filter(
name='Practical Django Projects', name='Practical Django Projects',
@ -218,7 +214,6 @@ class AggregationTests(TestCase):
{'pages__sum': 3703} {'pages__sum': 3703}
) )
@ignore_warnings(category=RemovedInDjango31Warning)
def test_annotation(self): def test_annotation(self):
# Annotations get combined with extra select clauses # Annotations get combined with extra select clauses
obj = Book.objects.annotate(mean_auth_age=Avg("authors__age")).extra( 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 # If an annotation isn't included in the values, it can still be used
# in a filter # 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( self.assertSequenceEqual(
qs, [ qs, [
{"name": 'Python Web Development with Django'} {"name": 'Python Web Development with Django'}
@ -457,7 +451,6 @@ class AggregationTests(TestCase):
with self.assertRaisesMessage(FieldError, msg): with self.assertRaisesMessage(FieldError, msg):
Book.objects.all().annotate(num_authors=Count('authors__id')).aggregate(Max('foo')) Book.objects.all().annotate(num_authors=Count('authors__id')).aggregate(Max('foo'))
@ignore_warnings(category=RemovedInDjango31Warning)
def test_more(self): def test_more(self):
# Old-style count aggregations can be mixed with new-style # Old-style count aggregations can be mixed with new-style
self.assertEqual( self.assertEqual(
@ -811,7 +804,6 @@ class AggregationTests(TestCase):
with self.assertRaisesMessage(ValueError, msg): with self.assertRaisesMessage(ValueError, msg):
Author.objects.annotate(book_contact_set=Avg('friends__age')) Author.objects.annotate(book_contact_set=Avg('friends__age'))
@ignore_warnings(category=RemovedInDjango31Warning)
def test_pickle(self): def test_pickle(self):
# Regression for #10197 -- Queries with aggregates can be pickled. # Regression for #10197 -- Queries with aggregates can be pickled.
# First check that pickling is possible at all. No crash = success # First check that pickling is possible at all. No crash = success
@ -1210,7 +1202,6 @@ class AggregationTests(TestCase):
{'book__count__max': 2} {'book__count__max': 2}
) )
@ignore_warnings(category=RemovedInDjango31Warning)
def test_annotate_joins(self): def test_annotate_joins(self):
""" """
The base table's join isn't promoted to LOUTER. This could The base table's join isn't promoted to LOUTER. This could
@ -1474,7 +1465,6 @@ class AggregationTests(TestCase):
query.filter(q1 | q2) query.filter(q1 | q2)
self.assertEqual(len(q2.children), 1) self.assertEqual(len(q2.children), 1)
@ignore_warnings(category=RemovedInDjango31Warning)
def test_fobj_group_by(self): def test_fobj_group_by(self):
""" """
An F() object referring to related column works correctly in group by. 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')) qs = Charlie.objects.annotate(Count('alfa__name'))
self.assertIn(' LEFT OUTER JOIN ', str(qs.query)) self.assertIn(' LEFT OUTER JOIN ', str(qs.query))
@ignore_warnings(category=RemovedInDjango31Warning)
def test_non_nullable_fk_not_promoted(self): def test_non_nullable_fk_not_promoted(self):
qs = Book.objects.annotate(Count('contact__name')) qs = Book.objects.annotate(Count('contact__name'))
self.assertIn(' INNER JOIN ', str(qs.query)) self.assertIn(' INNER JOIN ', str(qs.query))

View File

@ -3,11 +3,10 @@ from operator import attrgetter
from django.core.exceptions import FieldError from django.core.exceptions import FieldError
from django.db.models import ( 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.db.models.functions import Upper
from django.test import TestCase from django.test import TestCase
from django.utils.deprecation import RemovedInDjango31Warning
from .models import Article, Author, ChildArticle, OrderedByFArticle, Reference 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)) ca4 = ChildArticle.objects.create(headline='h1', pub_date=datetime(2005, 7, 28))
articles = ChildArticle.objects.order_by('article_ptr') articles = ChildArticle.objects.order_by('article_ptr')
self.assertSequenceEqual(articles, [ca4, ca2, ca1, ca3]) 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')))

View File

@ -2,11 +2,8 @@ import unittest
from django.db import NotSupportedError, connection, transaction from django.db import NotSupportedError, connection, transaction
from django.db.models import Count from django.db.models import Count
from django.test import ( from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature
TestCase, ignore_warnings, skipIfDBFeature, skipUnlessDBFeature,
)
from django.test.utils import CaptureQueriesContext from django.test.utils import CaptureQueriesContext
from django.utils.deprecation import RemovedInDjango31Warning
from .models import Tag from .models import Tag
@ -14,7 +11,6 @@ from .models import Tag
@skipUnlessDBFeature('supports_explaining_query_execution') @skipUnlessDBFeature('supports_explaining_query_execution')
class ExplainTests(TestCase): class ExplainTests(TestCase):
@ignore_warnings(category=RemovedInDjango31Warning)
def test_basic(self): def test_basic(self):
querysets = [ querysets = [
Tag.objects.filter(name='test'), Tag.objects.filter(name='test'),