Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#12822 closed (fixed)

DatabaseError: aggregates not allowed in WHERE clause

Reported by: Mathieu Pillard Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

From django-users ; I have been searching for an existing bug and couldn't find any in the ORM/ORM Aggregation component.

The following code works with django 1.1, and returns the expected results, but fails with 1.2 beta 1. It's a basic messaging system, in which you can group messages by
conversations : when saving a new Foo object, you can give it an existing Foo id to form a conversation. I want to display the "inbox"
for a user, which should be a list with the last message from each conversation.

from django.db import models
from django.contrib.auth.models import User

class Foo(models.Model):
   subject = models.CharField(max_length=120)
   sender = models.ForeignKey(User, related_name='sent_foo')
   recipient = models.ForeignKey(User, related_name='received_foo')
   conversation = models.ForeignKey('self', null=True, blank=True)

from django.db.models import Max

def conversations(self, user):
   tmp = Foo.objects.values('conversation').annotate(Max('id')).values_list('id__max', flat=True).order_by( 'conversation')
   return Foo.objects.filter(id__in=tmp.filter(recipient=user))

In 1.2 beta 1, with postgresql_psycopg2, it fails with:

DatabaseError: aggregates not allowed in WHERE clause
LINE 1: ...d" FROM "mat_foo" WHERE "mat_foo"."id" IN (SELECT MAX("mat_f...

The generated SQL queries are a bit different.
Here is django 1.2:

SELECT "mat_foo"."id", "mat_foo"."subject", "mat_foo"."sender_id", "mat_foo"."recipient_id", "mat_foo"."conversation_id" FROM "mat_foo"
WHERE "mat_foo"."id" IN (SELECT MAX("mat_foo"."id") AS "id__max" FROM "mat_foo" U0 WHERE U0."recipient_id" = 1  GROUP BY U0."conversation_id")

And here is django 1.1 (which works):

SELECT "mat_foo"."id", "mat_foo"."subject", "mat_foo"."sender_id", "mat_foo"."recipient_id", "mat_foo"."conversation_id" FROM "mat_foo"
WHERE "mat_foo"."id" IN (SELECT MAX(U0."id") AS "id__max" FROM "mat_foo" U0 WHERE U0."recipient_id" = 1  GROUP BY U0."conversation_id")

Note the difference in the MAX() clause. For what it's worth, it does work with sqlite. Didn't test other databases. I suspect there is a better way to write my query, but in any case, I don't think it should fail like that, especially since it was working with 1.1.

Attachments (3)

more-aggregation-unittests.diff (836 bytes) - added by Mathieu Pillard 7 years ago.
Unit test (without the hardcoded id this time)
ticket_12822_test.diff (1.8 KB) - added by Tobias McNulty 7 years ago.
updated tests demonstrating the actual issue
ticket_12822_tests_with_fix.diff (3.0 KB) - added by Tobias McNulty 7 years ago.
patch with tests + fix

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

Changed 7 years ago by Mathieu Pillard

Unit test (without the hardcoded id this time)

comment:2 Changed 7 years ago by Mathieu Pillard

I've tried to come up with a unit test for this bug and found surprising results. I've attached a patch adding my tests against the latest svn and it fails with sqlite too, but not with a DatabaseError : the results seem simply wrong.

Should I open another bug ? Either I'm completely wrong or it's a serious aggregation bug ; It passes correctly with django 1.1.

Note that with postgresql_psycopg2 the test does fail with the DatabaseError, so the original bug is still present.

comment:3 Changed 7 years ago by Mathieu Pillard

Version: 1.2-betaSVN

comment:4 Changed 7 years ago by Tobias McNulty

Owner: set to Tobias McNulty
Status: newassigned

comment:5 Changed 7 years ago by Tobias McNulty

Okay.. so the issue is more complex than originally thought. It does exist in 1.1, but was hidden in certain cases (namely, when the queryset to be used as a subquery was not evaluated prior to being used as a subquery). Attaching a patch with two tests demonstrating the issue. In 1.1, the first test passes, while the second one fails. In 1.2, both tests fail.

Changed 7 years ago by Tobias McNulty

Attachment: ticket_12822_test.diff added

updated tests demonstrating the actual issue

Changed 7 years ago by Tobias McNulty

patch with tests + fix

comment:6 Changed 7 years ago by Tobias McNulty

Has patch: set
Owner: changed from Tobias McNulty to nobody
Status: assignednew
Triage Stage: AcceptedReady for checkin

comment:7 Changed 7 years ago by Tobias McNulty

Owner: changed from nobody to Tobias McNulty
Status: newassigned

Whoops, need a 1.1 fix as well...

comment:8 Changed 7 years ago by Tobias McNulty

Owner: changed from Tobias McNulty to nobody
Status: assignednew

same patch applies to 1.1.X branch and fixes the issue there as well

comment:9 Changed 7 years ago by Karen Tracey

Resolution: fixed
Status: newclosed

(In [12830]) Fixed #12822: Don't copy the _aggregate_select_cache when cloning a query set,
as that can lead to incorrect SQL being generated for the query. Thanks to mat
for the report and test, tobias for the fix, and Alex for review.

comment:10 Changed 7 years ago by Karen Tracey

(In [12831]) [1.1.X] Fixed #12822: Don't copy the _aggregate_select_cache when cloning a query set,
as that can lead to incorrect SQL being generated for the query. Thanks to mat
for the report and test, tobias for the fix, and Alex for review.

r12830 from trunk.

comment:11 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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

Component: ORM aggregationDatabase layer (models, ORM)
Note: See TracTickets for help on using tickets.
Back to Top