Opened 5 years ago

Closed 5 years ago

Last modified 2 years ago

#12822 closed (fixed)

DatabaseError: aggregates not allowed in WHERE clause

Reported by: mat 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 mat 5 years ago.
Unit test (without the hardcoded id this time)
ticket_12822_test.diff (1.8 KB) - added by tobias 5 years ago.
updated tests demonstrating the actual issue
ticket_12822_tests_with_fix.diff (3.0 KB) - added by tobias 5 years ago.
patch with tests + fix

Download all attachments as: .zip

Change History (15)

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 5 years ago by mat

Unit test (without the hardcoded id this time)

comment:2 Changed 5 years ago by mat

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 5 years ago by mat

  • Version changed from 1.2-beta to SVN

comment:4 Changed 5 years ago by tobias

  • Owner set to tobias
  • Status changed from new to assigned

comment:5 Changed 5 years ago by tobias

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 5 years ago by tobias

updated tests demonstrating the actual issue

Changed 5 years ago by tobias

patch with tests + fix

comment:6 Changed 5 years ago by tobias

  • Has patch set
  • Owner changed from tobias to nobody
  • Status changed from assigned to new
  • Triage Stage changed from Accepted to Ready for checkin

comment:7 Changed 5 years ago by tobias

  • Owner changed from nobody to tobias
  • Status changed from new to assigned

Whoops, need a 1.1 fix as well...

comment:8 Changed 5 years ago by tobias

  • Owner changed from tobias to nobody
  • Status changed from assigned to new

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

comment:9 Changed 5 years ago by kmtracey

  • Resolution set to fixed
  • Status changed from new to closed

(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 5 years ago by kmtracey

(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 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

comment:12 Changed 2 years ago by akaariai

  • Component changed from ORM aggregation to Database layer (models, ORM)
Note: See TracTickets for help on using tickets.
Back to Top