Opened 14 years ago
Closed 11 years ago
#15624 closed Bug (fixed)
annotate + aggregate produces incorrect sql statement
Reported by: | Owned by: | fhahn | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | aggregate sql syntax error |
Cc: | flo@…, John Milner | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I have a following model:
class Solution(models.Model): griddler = models.ForeignKey(Griddler) user = models.ForeignKey(User) user_time = models.IntegerField(null=True) date = models.DateTimeField(auto_now_add=True) class Griddler(models.Model): .... solved_by = models.ManyToManyField(Uesr, through='Solution') ....
What I am trying to do is to find the Griddler that has the biggest
avg user time. I've written the following query:
Solution.objects.values('griddler').annotate(a=Avg('user_time')).aggregate( Max('a'))
Unfortunately, after executing it I get
DatabaseError: (1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FROM (SELECT griddlers_solution
.griddler_id
AS griddler_id
, AVG(`griddlers' at line 1")
>>> connection.queries[-1] {'time': '0.000', 'sql': u'SELECT FROM (SELECT `griddlers_solution`.`griddler_id` AS `griddler_id`, AVG(`griddlers_solution`.`user_time`) AS `a` FROM `griddlers_solution` GROUP BY `griddlers_solution`.`griddler_id`, `griddlers_solution`.`griddler_id` ORDER BY NULL) subquery'}
The problem is that Max('a') is skipped in the sql query. Instead of :
SELECT FROM (SELECT ...
It should look like this:
SELECT Max(`a`) FROM (SELECT...
I am using Django from SVN (pulled today, 16.03.11) and MySQL backend.
>>> django.get_version() '1.3 beta 1'
Attachments (3)
Change History (16)
comment:1 by , 14 years ago
Component: | Database layer (models, ORM) → ORM aggregation |
---|
comment:2 by , 14 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
by , 14 years ago
Attachment: | 15624.test.diff added |
---|
comment:4 by , 14 years ago
Type: | → Bug |
---|
comment:5 by , 14 years ago
Severity: | → Normal |
---|
by , 14 years ago
Attachment: | aggregate_bug_quick_patch.diff added |
---|
That's a quick hack that should work, but there probably exist more elegant solution.
by , 13 years ago
Attachment: | aggregate_bug_quick_patch2.diff added |
---|
Added more consistent version of the patch.
comment:6 by , 13 years ago
Easy pickings: | unset |
---|---|
Has patch: | set |
Needs tests: | set |
UI/UX: | unset |
comment:7 by , 13 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:8 by , 12 years ago
Hello! Any news on that? I see there's a patch, now oldish, but nothing happened since and nobody from core has commented on it... is that patch a viable solution?
comment:9 by , 12 years ago
Cc: | added |
---|---|
Needs tests: | unset |
Owner: | set to |
Status: | new → assigned |
I think the problem with Book.objects.annotate(c=Count('authors')).values('c').aggregate(Max('c'))
was that the call to .values('c')
set the aggregates_select_mask
to set(['c'])
and then c__max
does not show up in aggregate_select
.
My patch adds the alias of the aggregate to aggregate_select_mask
, if aggregate_select_mask
is not None.
I've updated the patch and created a pull request: https://github.com/django/django/pull/738
comment:10 by , 12 years ago
Component: | ORM aggregation → Database layer (models, ORM) |
---|
comment:11 by , 11 years ago
Cc: | added |
---|
comment:12 by , 11 years ago
Summary: | aggregate produces incorrect sql statement → annotate + aggregate produces incorrect sql statement |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Patch applies cleanly and tests pass. Would like someone more familiar with the code to +1 before committing.
comment:13 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Attached patch with a unit test confirming this issue.