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).
This commit is contained in:
Anssi Kääriäinen 2015-05-05 14:44:33 +03:00 committed by Tim Graham
parent be9d645346
commit adc57632bc
4 changed files with 26 additions and 3 deletions

View File

@ -143,9 +143,12 @@ class SQLCompiler(object):
# then also add having expressions to group by. # then also add having expressions to group by.
pk = None pk = None
for expr in expressions: for expr in expressions:
if (expr.output_field.primary_key and # Is this a reference to query's base table primary key? If the
getattr(expr.output_field, 'model') == self.query.model): # 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 pk = expr
break
if pk: if pk:
# MySQLism: Columns in HAVING clause must be added to the GROUP BY. # MySQLism: Columns in HAVING clause must be added to the GROUP BY.
expressions = [pk] + [expr for expr in expressions if expr in having] expressions = [pk] + [expr for expr in expressions if expr in having]

View File

@ -17,3 +17,6 @@ Bugfixes
* Corrected join promotion for ``Case`` expressions. For example, annotating a * Corrected join promotion for ``Case`` expressions. For example, annotating a
query with a ``Case`` expression could unexpectedly filter out results query with a ``Case`` expression could unexpectedly filter out results
(:ticket:`24766`). (:ticket:`24766`).
* Fixed incorrect GROUP BY clause generation on MySQL when the query's model
has a self-referential foreign key (:ticket:`24748`).

View File

@ -104,3 +104,8 @@ class Bravo(models.Model):
class Charlie(models.Model): class Charlie(models.Model):
alfa = models.ForeignKey(Alfa, null=True) alfa = models.ForeignKey(Alfa, null=True)
bravo = models.ForeignKey(Bravo, 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')

View File

@ -17,7 +17,7 @@ from django.utils import six
from .models import ( from .models import (
Alfa, Author, Book, Bravo, Charlie, Clues, Entries, HardbackBook, ItemTag, Alfa, Author, Book, Bravo, Charlie, Clues, Entries, HardbackBook, ItemTag,
Publisher, Store, WithManualPK, Publisher, SelfRefFK, Store, WithManualPK,
) )
@ -1277,3 +1277,15 @@ class JoinPromotionTests(TestCase):
def test_non_nullable_fk_not_promoted(self): def test_non_nullable_fk_not_promoted(self):
qs = Book.objects.annotate(Count('contact__name')) qs = Book.objects.annotate(Count('contact__name'))
self.assertIn(' INNER JOIN ', str(qs.query)) self.assertIn(' INNER JOIN ', str(qs.query))
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)
)