From 2d76b61dc2a45d5acda53736433a22a1b74e0514 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Thu, 23 Apr 2015 20:13:24 -0400 Subject: [PATCH] Fixed #24649 -- Allowed using Avg aggregate on non-numeric field types. --- django/db/backends/base/features.py | 3 +++ django/db/backends/oracle/features.py | 1 + django/db/models/aggregates.py | 8 ++----- docs/ref/models/querysets.txt | 13 ++++++++--- docs/releases/1.9.txt | 4 ++++ tests/aggregation/models.py | 1 + tests/aggregation/tests.py | 31 +++++++++++++++++++++------ 7 files changed, 46 insertions(+), 15 deletions(-) diff --git a/django/db/backends/base/features.py b/django/db/backends/base/features.py index d9463ddf19..e03a097a04 100644 --- a/django/db/backends/base/features.py +++ b/django/db/backends/base/features.py @@ -160,6 +160,9 @@ class BaseDatabaseFeatures(object): # Support for the DISTINCT ON clause can_distinct_on_fields = False + # Can the backend use an Avg aggregate on DurationField? + can_avg_on_durationfield = True + # Does the backend decide to commit before SAVEPOINT statements # when autocommit is disabled? http://bugs.python.org/issue8145#msg109965 autocommits_when_autocommit_is_off = False diff --git a/django/db/backends/oracle/features.py b/django/db/backends/oracle/features.py index 0b152e612e..2a17bc5a8f 100644 --- a/django/db/backends/oracle/features.py +++ b/django/db/backends/oracle/features.py @@ -40,6 +40,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): uppercases_column_names = True # select for update with limit can be achieved on Oracle, but not with the current backend. supports_select_for_update_with_limit = False + can_avg_on_durationfield = False # Pending implementation (#24699). def introspected_boolean_field_type(self, field=None, created_separately=False): """ diff --git a/django/db/models/aggregates.py b/django/db/models/aggregates.py index b51fe5635a..2d7c43c90e 100644 --- a/django/db/models/aggregates.py +++ b/django/db/models/aggregates.py @@ -75,12 +75,8 @@ class Avg(Aggregate): name = 'Avg' def __init__(self, expression, **extra): - super(Avg, self).__init__(expression, output_field=FloatField(), **extra) - - def convert_value(self, value, expression, connection, context): - if value is None: - return value - return float(value) + output_field = extra.pop('output_field', FloatField()) + super(Avg, self).__init__(expression, output_field=output_field, **extra) class Count(Aggregate): diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 3dc66f631a..d2cbc63203 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -2802,12 +2802,19 @@ by the aggregate. Avg ~~~ -.. class:: Avg(expression, output_field=None, **extra) +.. class:: Avg(expression, output_field=FloatField(), **extra) - Returns the mean value of the given expression, which must be numeric. + Returns the mean value of the given expression, which must be numeric + unless you specify a different ``output_field``. * Default alias: ``__avg`` - * Return type: ``float`` + * Return type: ``float`` (or the type of whatever ``output_field`` is + specified) + + .. versionchanged:: 1.9 + + The ``output_field`` parameter was added to allow aggregating over + non-numeric columns, such as ``DurationField``. Count ~~~~~ diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 525cf34630..21df59f051 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -200,6 +200,10 @@ Models (such as :lookup:`exact`, :lookup:`gt`, :lookup:`lt`, etc.). For example: ``Entry.objects.filter(pub_date__month__gt=6)``. +* You can specify the ``output_field`` parameter of the + :class:`~django.db.models.Avg` aggregate in order to aggregate over + non-numeric columns, such as ``DurationField``. + CSRF ^^^^ diff --git a/tests/aggregation/models.py b/tests/aggregation/models.py index 0ca20def3e..de9dc5550c 100644 --- a/tests/aggregation/models.py +++ b/tests/aggregation/models.py @@ -17,6 +17,7 @@ class Author(models.Model): class Publisher(models.Model): name = models.CharField(max_length=255) num_awards = models.IntegerField() + duration = models.DurationField(blank=True, null=True) def __str__(self): return self.name diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index 3e2bc2eb30..81152fd2ad 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -7,10 +7,10 @@ from decimal import Decimal from django.core.exceptions import FieldError from django.db import connection from django.db.models import ( - F, Aggregate, Avg, Count, DecimalField, FloatField, Func, IntegerField, - Max, Min, Sum, Value, + F, Aggregate, Avg, Count, DecimalField, DurationField, FloatField, Func, + IntegerField, Max, Min, Sum, Value, ) -from django.test import TestCase, ignore_warnings +from django.test import TestCase, ignore_warnings, skipUnlessDBFeature from django.test.utils import Approximate, CaptureQueriesContext from django.utils import six, timezone from django.utils.deprecation import RemovedInDjango20Warning @@ -40,8 +40,8 @@ class AggregateTestCase(TestCase): cls.a8.friends.add(cls.a9) cls.a9.friends.add(cls.a8) - cls.p1 = Publisher.objects.create(name='Apress', num_awards=3) - cls.p2 = Publisher.objects.create(name='Sams', num_awards=1) + cls.p1 = Publisher.objects.create(name='Apress', num_awards=3, duration=datetime.timedelta(days=1)) + cls.p2 = Publisher.objects.create(name='Sams', num_awards=1, duration=datetime.timedelta(days=2)) cls.p3 = Publisher.objects.create(name='Prentice Hall', num_awards=7) cls.p4 = Publisher.objects.create(name='Morgan Kaufmann', num_awards=9) cls.p5 = Publisher.objects.create(name="Jonno's House of Books", num_awards=0) @@ -441,6 +441,13 @@ class AggregateTestCase(TestCase): vals = Book.objects.annotate(num_authors=Count("authors__id")).aggregate(Avg("num_authors")) self.assertEqual(vals, {"num_authors__avg": Approximate(1.66, places=1)}) + @skipUnlessDBFeature('can_avg_on_durationfield') + def test_avg_duration_field(self): + self.assertEqual( + Publisher.objects.aggregate(Avg('duration', output_field=DurationField())), + {'duration__avg': datetime.timedelta(1, 43200)} # 1.5 days + ) + def test_sum_distinct_aggregate(self): """ Sum on a distict() QuerySet should aggregate only the distinct items. @@ -620,7 +627,14 @@ class AggregateTestCase(TestCase): self.assertEqual(vals, {"rating__avg": 4.25}) def test_even_more_aggregate(self): - publishers = Publisher.objects.annotate(earliest_book=Min("book__pubdate")).exclude(earliest_book=None).order_by("earliest_book").values() + publishers = Publisher.objects.annotate( + earliest_book=Min("book__pubdate"), + ).exclude(earliest_book=None).order_by("earliest_book").values( + 'earliest_book', + 'num_awards', + 'id', + 'name', + ) self.assertEqual( list(publishers), [ { @@ -836,6 +850,11 @@ class AggregateTestCase(TestCase): self.assertEqual(a2, {'av_age': 37}) self.assertEqual(a3, {'av_age': Approximate(37.4, places=1)}) + def test_avg_decimal_field(self): + v = Book.objects.filter(rating=4).aggregate(avg_price=(Avg('price')))['avg_price'] + self.assertIsInstance(v, float) + self.assertEqual(v, Approximate(47.39, places=2)) + def test_order_of_precedence(self): p1 = Book.objects.filter(rating=4).aggregate(avg_price=(Avg('price') + 2) * 3) self.assertEqual(p1, {'avg_price': Approximate(148.18, places=2)})