From 1d070d027c218285b66c0bde8079034b33a87f11 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 29 Mar 2017 06:47:07 +0200 Subject: [PATCH] Fixed #25414 -- Fixed QuerySet.annotate() with pk in values() on MySQL. Thanks Tim Graham and Simon Charette for the reviews. --- django/db/models/sql/compiler.py | 17 ++++++++++++----- tests/annotations/tests.py | 6 ++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 3215aaaa9c..b274ba24ae 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -135,9 +135,7 @@ class SQLCompiler: # include the primary key of every table, but for MySQL it is enough to # have the main table's primary key. if self.connection.features.allows_group_by_pk: - # The logic here is: if the main model's primary key is in the - # query, then set new_expressions to that field. If that happens, - # then also add having expressions to group by. + # Determine if the main model's primary key is in the query. pk = None for expr in expressions: # Is this a reference to query's base table primary key? If the @@ -146,9 +144,18 @@ class SQLCompiler: getattr(expr, 'alias', None) == self.query.tables[0]): pk = expr break + # If the main model's primary key is in the query, group by that + # field, HAVING expressions, and expressions associated with tables + # that don't have a primary key included in the grouped columns. if pk: - # MySQLism: Columns in HAVING clause must be added to the GROUP BY. - expressions = [pk] + [expr for expr in expressions if expr in having] + pk_aliases = { + expr.alias for expr in expressions + if hasattr(expr, 'target') and expr.target.primary_key + } + expressions = [pk] + [ + expr for expr in expressions + if expr in having or getattr(expr, 'alias', None) not in pk_aliases + ] elif self.connection.features.allows_group_by_selected_pks: # Filter out all expressions associated with a table's primary key # present in the grouped columns. This is done by identifying all diff --git a/tests/annotations/tests.py b/tests/annotations/tests.py index b149ddf46c..2ad395e65c 100644 --- a/tests/annotations/tests.py +++ b/tests/annotations/tests.py @@ -297,6 +297,12 @@ class NonAggregateAnnotationTestCase(TestCase): self.assertEqual(book['other_rating'], 4) self.assertEqual(book['other_isbn'], '155860191') + def test_values_with_pk_annotation(self): + # annotate references a field in values() with pk + publishers = Publisher.objects.values('id', 'book__rating').annotate(total=Sum('book__rating')) + for publisher in publishers.filter(pk=self.p1.pk): + self.assertEqual(publisher['book__rating'], publisher['total']) + def test_defer_annotation(self): """ Deferred attributes can be referenced by an annotation,