From d5ce2dd7bc5b797d2e338c4bc6e6f3e339b748e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Tue, 5 May 2015 14:44:33 +0300 Subject: [PATCH] [1.8.x] Fixed #24748 -- Fixed incorrect GROUP BY on MySQL in some queries When the query's model had a self-referential foreign key, the compiler.get_group_by() code incorrectly used the self-referential foreign key's column (for example parent_id) as GROUP BY clause when it should have used the model's primary key column (id). Backport of adc57632bc26cc8fe42bdb6aff463f883214980a from master --- django/db/models/sql/compiler.py | 7 ++++-- docs/releases/1.8.2.txt | 3 +++ tests/aggregation_regress/models.py | 5 +++++ tests/aggregation_regress/tests.py | 35 +++++++++-------------------- 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 810031f47ef..6aab2045988 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -145,9 +145,12 @@ class SQLCompiler(object): # then also add having expressions to group by. pk = None for expr in expressions: - if (expr.output_field.primary_key and - getattr(expr.output_field, 'model') == self.query.model): + # Is this a reference to query's base table primary key? If the + # expression isn't a Col-like, then skip the expression. + if (getattr(expr, 'target', None) == self.query.model._meta.pk and + getattr(expr, 'alias', None) == self.query.tables[0]): pk = expr + break if pk: expressions = [pk] + [expr for expr in expressions if expr in having] return expressions diff --git a/docs/releases/1.8.2.txt b/docs/releases/1.8.2.txt index f430eb6cb0b..bf54aeefd40 100644 --- a/docs/releases/1.8.2.txt +++ b/docs/releases/1.8.2.txt @@ -17,3 +17,6 @@ Bugfixes * Corrected join promotion for ``Case`` expressions. For example, annotating a query with a ``Case`` expression could unexpectedly filter out results (:ticket:`24766`). + +* Fixed incorrect GROUP BY clause generation on MySQL when the query's model + has a self-referential foreign key (:ticket:`24748`). diff --git a/tests/aggregation_regress/models.py b/tests/aggregation_regress/models.py index 2cad0a54865..4d675952c2c 100644 --- a/tests/aggregation_regress/models.py +++ b/tests/aggregation_regress/models.py @@ -104,3 +104,8 @@ class Bravo(models.Model): class Charlie(models.Model): alfa = models.ForeignKey(Alfa, null=True) bravo = models.ForeignKey(Bravo, null=True) + + +class SelfRefFK(models.Model): + name = models.CharField(max_length=50) + parent = models.ForeignKey('self', null=True, blank=True, related_name='children') diff --git a/tests/aggregation_regress/tests.py b/tests/aggregation_regress/tests.py index bc6a55875d9..12bb1ae3f4c 100644 --- a/tests/aggregation_regress/tests.py +++ b/tests/aggregation_regress/tests.py @@ -16,7 +16,7 @@ from django.utils import six from .models import ( Alfa, Author, Book, Bravo, Charlie, Clues, Entries, HardbackBook, ItemTag, - Publisher, WithManualPK, + Publisher, SelfRefFK, WithManualPK, ) @@ -1174,28 +1174,13 @@ class JoinPromotionTests(TestCase): self.assertIn(' INNER JOIN ', str(qs.query)) -class AggregationOnRelationTest(TestCase): - def setUp(self): - self.a = Author.objects.create(name='Anssi', age=33) - self.p = Publisher.objects.create(name='Manning', num_awards=3) - Book.objects.create(isbn='asdf', name='Foo', pages=10, rating=0.1, price="0.0", - contact=self.a, publisher=self.p, pubdate=datetime.date.today()) - - def test_annotate_on_relation(self): - qs = Book.objects.annotate(avg_price=Avg('price'), publisher_name=F('publisher__name')) - self.assertEqual(qs[0].avg_price, 0.0) - self.assertEqual(qs[0].publisher_name, "Manning") - - def test_aggregate_on_relation(self): - # A query with an existing annotation aggregation on a relation should - # succeed. - qs = Book.objects.annotate(avg_price=Avg('price')).aggregate( - publisher_awards=Sum('publisher__num_awards') +class SelfReferentialFKTests(TestCase): + def test_ticket_24748(self): + t1 = SelfRefFK.objects.create(name='t1') + SelfRefFK.objects.create(name='t2', parent=t1) + SelfRefFK.objects.create(name='t3', parent=t1) + self.assertQuerysetEqual( + SelfRefFK.objects.annotate(num_children=Count('children')).order_by('name'), + [('t1', 2), ('t2', 0), ('t3', 0)], + lambda x: (x.name, x.num_children) ) - self.assertEqual(qs['publisher_awards'], 3) - Book.objects.create(isbn='asdf', name='Foo', pages=10, rating=0.1, price="0.0", - contact=self.a, publisher=self.p, pubdate=datetime.date.today()) - qs = Book.objects.annotate(avg_price=Avg('price')).aggregate( - publisher_awards=Sum('publisher__num_awards') - ) - self.assertEqual(qs['publisher_awards'], 6)