From 98ef7e85bdaad4b21c31b425a62b2c8bc294be48 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Wed, 8 Apr 2009 13:19:48 +0000 Subject: [PATCH] Fixed #10666 -- Corrected the handling of inherited fields with aggregate() and annotate(). Thanks to julienb for the report. git-svn-id: http://code.djangoproject.com/svn/django/trunk@10446 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/sql/query.py | 24 +++++++++------- .../fixtures/initial_data.json | 14 ++++++++++ .../aggregation_regress/models.py | 28 +++++++++++++++++-- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index cafde11415..256cbef8e1 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1403,8 +1403,17 @@ class BaseQuery(object): field_name = field_list[0] col = field_name source = self.aggregates[field_name] - elif (len(field_list) > 1 or - field_list[0] not in [i.name for i in opts.fields]): + elif ((len(field_list) > 1) or + (field_list[0] not in [i.name for i in opts.fields]) or + self.group_by is None or + not is_summary): + # If: + # - the field descriptor has more than one part (foo__bar), or + # - the field descriptor is referencing an m2m/m2o field, or + # - this is a reference to a model field (possibly inherited), or + # - this is an annotation over a model field + # then we need to explore the joins that are required. + field, source, opts, join_list, last, _ = self.setup_joins( field_list, opts, self.get_initial_alias(), False) @@ -1419,15 +1428,11 @@ class BaseQuery(object): col = (join_list[-1], col) else: - # Aggregate references a normal field + # The simplest cases. No joins required - + # just reference the provided column alias. field_name = field_list[0] source = opts.get_field(field_name) - if not (self.group_by is not None and is_summary): - # Only use a column alias if this is a - # standalone aggregate, or an annotation - col = (opts.db_table, source.column) - else: - col = field_name + col = field_name # Add the aggregate to the query alias = truncate_name(alias, self.connection.ops.max_name_length()) @@ -1659,7 +1664,6 @@ class BaseQuery(object): last.append(len(joins)) if name == 'pk': name = opts.pk.name - try: field, model, direct, m2m = opts.get_field_by_name(name) except FieldDoesNotExist: diff --git a/tests/regressiontests/aggregation_regress/fixtures/initial_data.json b/tests/regressiontests/aggregation_regress/fixtures/initial_data.json index 9fb88572b0..597ac041f7 100644 --- a/tests/regressiontests/aggregation_regress/fixtures/initial_data.json +++ b/tests/regressiontests/aggregation_regress/fixtures/initial_data.json @@ -239,5 +239,19 @@ "friends": [8], "name": "Stuart Russell" } + }, + { + "pk": 5, + "model": "aggregation_regress.hardbackbook", + "fields": { + "weight": 4.5 + } + }, + { + "pk": 6, + "model": "aggregation_regress.hardbackbook", + "fields": { + "weight": 3.7 + } } ] diff --git a/tests/regressiontests/aggregation_regress/models.py b/tests/regressiontests/aggregation_regress/models.py index 2dfa1818fc..3ce0f6e75d 100644 --- a/tests/regressiontests/aggregation_regress/models.py +++ b/tests/regressiontests/aggregation_regress/models.py @@ -58,6 +58,12 @@ class Clues(models.Model): EntryID = models.ForeignKey(Entries, verbose_name='Entry', db_column = 'Entry ID') Clue = models.CharField(max_length=150) +class HardbackBook(Book): + weight = models.FloatField() + + def __unicode__(self): + return "%s (hardback): %s" % (self.name, self.weight) + __test__ = {'API_TESTS': """ >>> from django.core import management >>> from django.db.models import get_app, F @@ -137,17 +143,17 @@ __test__ = {'API_TESTS': """ >>> Book.objects.all().aggregate(num_authors=Count('foo')) Traceback (most recent call last): ... -FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, id, isbn, name, pages, price, pubdate, publisher, rating, store +FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, hardbackbook, id, isbn, name, pages, price, pubdate, publisher, rating, store >>> Book.objects.all().annotate(num_authors=Count('foo')) Traceback (most recent call last): ... -FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, id, isbn, name, pages, price, pubdate, publisher, rating, store +FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, hardbackbook, id, isbn, name, pages, price, pubdate, publisher, rating, store >>> Book.objects.all().annotate(num_authors=Count('authors__id')).aggregate(Max('foo')) Traceback (most recent call last): ... -FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, id, isbn, name, pages, price, pubdate, publisher, rating, store, num_authors +FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, contact, hardbackbook, id, isbn, name, pages, price, pubdate, publisher, rating, store, num_authors # Old-style count aggregations can be mixed with new-style >>> Book.objects.annotate(num_authors=Count('authors')).count() @@ -276,6 +282,22 @@ FieldError: Cannot resolve keyword 'foo' into field. Choices are: authors, conta >>> publishers [, ] + + +# Regression for 10666 - inherited fields work with annotations and aggregations +>>> HardbackBook.objects.aggregate(n_pages=Sum('book_ptr__pages')) +{'n_pages': 2078} + +>>> HardbackBook.objects.aggregate(n_pages=Sum('pages')) +{'n_pages': 2078} + +>>> HardbackBook.objects.annotate(n_authors=Count('book_ptr__authors')).values('name','n_authors') +[{'n_authors': 2, 'name': u'Artificial Intelligence: A Modern Approach'}, {'n_authors': 1, 'name': u'Paradigms of Artificial Intelligence Programming: Case Studies in Common Lisp'}] + +>>> HardbackBook.objects.annotate(n_authors=Count('authors')).values('name','n_authors') +[{'n_authors': 2, 'name': u'Artificial Intelligence: A Modern Approach'}, {'n_authors': 1, 'name': u'Paradigms of Artificial Intelligence Programming: Case Studies in Common Lisp'}] + + """ }