Fixed #24649 -- Allowed using Avg aggregate on non-numeric field types.

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: ``<field>__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)})