From 0580133971e42c322a9c0b451d3bbf72eff2ca24 Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Wed, 21 Jan 2015 12:47:49 +0700 Subject: [PATCH] [1.8.x] Fixed small inconsistency when handling aggregate's default_alias. Refs #14030. Backport of d450af8a26 from master --- django/db/models/query.py | 16 ++++++++++++---- tests/aggregation/tests.py | 4 +++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 6774feef0c..2a1dbf9472 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -288,7 +288,13 @@ class QuerySet(object): if self.query.distinct_fields: raise NotImplementedError("aggregate() + distinct(fields) not implemented.") for arg in args: - if not hasattr(arg, 'default_alias'): + # The default_alias property may raise a TypeError, so we use + # a try/except construct rather than hasattr in order to remain + # consistent between PY2 and PY3 (hasattr would swallow + # the TypeError on PY2). + try: + arg.default_alias + except (AttributeError, TypeError): raise TypeError("Complex aggregates require an alias") kwargs[arg.default_alias] = arg @@ -759,14 +765,16 @@ class QuerySet(object): """ annotations = OrderedDict() # To preserve ordering of args for arg in args: + # The default_alias property may raise a TypeError, so we use + # a try/except construct rather than hasattr in order to remain + # consistent between PY2 and PY3 (hasattr would swallow + # the TypeError on PY2). try: - # we can't do an hasattr here because py2 returns False - # if default_alias exists but throws a TypeError if arg.default_alias in kwargs: raise ValueError("The named annotation '%s' conflicts with the " "default name for another annotation." % arg.default_alias) - except AttributeError: # default_alias + except (AttributeError, TypeError): raise TypeError("Complex annotations require an alias") annotations[arg.default_alias] = arg annotations.update(kwargs) diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py index ec19fcfd53..7e500dfb0e 100644 --- a/tests/aggregation/tests.py +++ b/tests/aggregation/tests.py @@ -768,10 +768,12 @@ class ComplexAggregateTestCase(TestCase): self.assertEqual(b3.sums, Approximate(Decimal("383.69"), places=2)) def test_complex_aggregations_require_kwarg(self): - with six.assertRaisesRegex(self, TypeError, 'Complex expressions require an alias'): + with six.assertRaisesRegex(self, TypeError, 'Complex annotations require an alias'): Author.objects.annotate(Sum(F('age') + F('friends__age'))) with six.assertRaisesRegex(self, TypeError, 'Complex aggregates require an alias'): Author.objects.aggregate(Sum('age') / Count('age')) + with six.assertRaisesRegex(self, TypeError, 'Complex aggregates require an alias'): + Author.objects.aggregate(Sum(Value(1))) def test_aggregate_over_complex_annotation(self): qs = Author.objects.annotate(