From ae8ff5f476973d5623675460d36a2f3c6853f6d0 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Sun, 16 Jan 2011 08:03:25 +0000 Subject: [PATCH] Fixed #14707 -- Relax the protections on aggregate naming collisions when a ValuesQuerySet removes the colliding name. Thanks to Andy McKay for the report. git-svn-id: http://code.djangoproject.com/svn/django/trunk@15223 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/query.py | 9 ++++---- .../aggregation_regress/tests.py | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 477f04a964..3c43a57d4a 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -620,18 +620,19 @@ class QuerySet(object): """ for arg in args: if arg.default_alias in kwargs: - raise ValueError("The %s named annotation conflicts with the " + raise ValueError("The named annotation '%s' conflicts with the " "default name for another annotation." % arg.default_alias) kwargs[arg.default_alias] = arg - names = set(self.model._meta.get_all_field_names()) + names = getattr(self, '_fields', None) + if names is None: + names = set(self.model._meta.get_all_field_names()) for aggregate in kwargs: if aggregate in names: - raise ValueError("The %s annotation conflicts with a field on " + raise ValueError("The annotation '%s' conflicts with a field on " "the model." % aggregate) - obj = self._clone() obj._setup_aggregate_query(kwargs.keys()) diff --git a/tests/regressiontests/aggregation_regress/tests.py b/tests/regressiontests/aggregation_regress/tests.py index ec2603fe1e..989c4c3d90 100644 --- a/tests/regressiontests/aggregation_regress/tests.py +++ b/tests/regressiontests/aggregation_regress/tests.py @@ -474,6 +474,28 @@ class AggregationTests(TestCase): # Regression for #11256 - providing an aggregate name that conflicts with an m2m name on the model raises ValueError self.assertRaises(ValueError, Author.objects.annotate, friends=Count('friends')) + def test_values_queryset_non_conflict(self): + # Regression for #14707 -- If you're using a values query set, some potential conflicts are avoided. + + # age is a field on Author, so it shouldn't be allowed as an aggregate. + # But age isn't included in the ValuesQuerySet, so it is. + results = Author.objects.values('name').annotate(age=Count('book_contact_set')) + self.assertEquals(len(results), 9) + self.assertEquals(results[0]['name'], u'Adrian Holovaty') + self.assertEquals(results[0]['age'], 1) + + # Same problem, but aggregating over m2m fields + results = Author.objects.values('name').annotate(age=Avg('friends__age')) + self.assertEquals(len(results), 9) + self.assertEquals(results[0]['name'], u'Adrian Holovaty') + self.assertEquals(results[0]['age'], 32.0) + + # Same problem, but colliding with an m2m field + results = Author.objects.values('name').annotate(friends=Count('friends')) + self.assertEquals(len(results), 9) + self.assertEquals(results[0]['name'], u'Adrian Holovaty') + self.assertEquals(results[0]['friends'], 2) + def test_reverse_relation_name_conflict(self): # Regression for #11256 - providing an aggregate name that conflicts with a reverse-related name on the model raises ValueError self.assertRaises(ValueError, Author.objects.annotate, book_contact_set=Avg('friends__age'))