Opened 6 years ago

Closed 3 years ago

#15624 closed Bug (fixed)

annotate + aggregate produces incorrect sql statement

Reported by: zeroos <232002@…> Owned by: fhahn
Component: Database layer (models, ORM) Version: master
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)

15624.test.diff (938 bytes) - added by Matthias Kestenholz 6 years ago.
aggregate_bug_quick_patch.diff (869 bytes) - added by marekw2143 6 years ago.
That's a quick hack that should work, but there probably exist more elegant solution.
aggregate_bug_quick_patch2.diff (1.4 KB) - added by mwawrzyczek 5 years ago.
Added more consistent version of the patch.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 6 years ago by anonymous

Component: Database layer (models, ORM)ORM aggregation

comment:2 Changed 6 years ago by marekw2143

Owner: changed from nobody to marekw2143
Status: newassigned

comment:3 Changed 6 years ago by Matthias Kestenholz

Triage Stage: UnreviewedAccepted

Attached patch with a unit test confirming this issue.

Changed 6 years ago by Matthias Kestenholz

Attachment: 15624.test.diff added

comment:4 Changed 6 years ago by Luke Plant

Type: Bug

comment:5 Changed 6 years ago by Luke Plant

Severity: Normal

Changed 6 years ago by marekw2143

That's a quick hack that should work, but there probably exist more elegant solution.

Changed 5 years ago by mwawrzyczek

Added more consistent version of the patch.

comment:6 Changed 5 years ago by Ramiro Morales

Easy pickings: unset
Has patch: set
Needs tests: set
UI/UX: unset

comment:7 Changed 5 years ago by marekw2143

Owner: marekw2143 deleted
Status: assignednew

comment:8 Changed 4 years ago by stefano@…

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 Changed 4 years ago by fhahn

Cc: flo@… added
Needs tests: unset
Owner: set to fhahn
Status: newassigned

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

Last edited 4 years ago by fhahn (previous) (diff)

comment:10 Changed 4 years ago by Anssi Kääriäinen

Component: ORM aggregationDatabase layer (models, ORM)

comment:11 Changed 4 years ago by John Milner

Cc: John Milner added

comment:12 Changed 3 years ago by Tim Graham

Summary: aggregate produces incorrect sql statementannotate + aggregate produces incorrect sql statement
Triage Stage: AcceptedReady for checkin

Patch applies cleanly and tests pass. Would like someone more familiar with the code to +1 before committing.

comment:13 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

Resolution: fixed
Status: assignedclosed

In e888a9b30dd9d62a930b9244d1b5531bb17544b4:

Fixed #15624 -- Made sure aggregations are present in SELECT

Note: See TracTickets for help on using tickets.
Back to Top