#25095: Fields annotated for filtering which are not included in values are
included in GROUP BY clause in SQL
-------------------------------------+-------------------------------------
Reporter: mitchelljkotler | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jarshwah):
* needs_docs: => 0
* needs_better_patch: => 0
* version: 1.8 => master
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
I've replicated the problem:
{{{
class FTest(models.Model):
talk = models.IntegerField()
hold = models.IntegerField()
In [8]: from scratch.models import *; from django.db.models import F,
Count
In [9]:
print(FTest.objects.annotate(talking=F('talk')).filter(talking__lte=30).values('hold').annotate(thecount=Count('pk')).query)
SELECT "scratch_ftest"."hold", COUNT("scratch_ftest"."id") AS "thecount"
FROM "scratch_ftest" WHERE "scratch_ftest"."talk" <= 30 GROUP BY
"scratch_ftest"."hold", "scratch_ftest"."talk"
}}}
It seems like your proposed fix works for your case, but my test VM is
broken at the moment so I can't run the full test suite.
{{{
$ git diff
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
index df65405..2f2375d 100644
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -1674,8 +1674,8 @@ class Query(object):
for col in self.select:
self.group_by.append(col)
- if self._annotations:
- for alias, annotation in six.iteritems(self.annotations):
+ if self.annotation_select:
+ for alias, annotation in
six.iteritems(self.annotation_select):
for col in annotation.get_group_by_cols():
self.group_by.append(col)
}}}
With the patch applied:
{{{
In [2]:
print(FTest.objects.annotate(talking=F('talk')).filter(talking__lte=30).values('hold').annotate(thecount=Count('pk')).query)
SELECT "scratch_ftest"."hold", COUNT("scratch_ftest"."id") AS "thecount"
FROM "scratch_ftest" WHERE "scratch_ftest"."talk" <= 30 GROUP BY
"scratch_ftest"."hold"
}}}
To hopefully clear up the report somewhat, the issue is that all
annotations are added to the group by list, even if they aren't in the
select list. By applying the above patch, we only add the annotations in
the select list to the group by list.
I'd like to see a test that had an annotation in the .order_by() clause
that wasn't included in .values(), just to ensure we aren't removing too
much from the group by.
Thanks for the great report and the fix. If you'd like to open up a pull
request with the proposed change and some tests, that'd be very welcome.
--
Ticket URL: <https://code.djangoproject.com/ticket/25095#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/073.2f0a38093a9bbcd7c88cb93aa8ca1e12%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
