Opened 16 years ago

Closed 16 years ago

Last modified 13 years ago

#10962 closed (invalid)

GROUP BY generation and extra seems to be too greedy

Reported by: Brian Rosner Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I may not understand the ORM code fully, but this seems like wrong behavior. Take the following queryset:

queryset = channel.message_set.all().values(
   "nickname",
).annotate(
   message_count = models.Count("id"),
).extra(select={
    "percentage": "FLOOR((COUNT(*) / %d.0) * 100)" % channel.message_set.count(),
}).order_by("-message_count")

I am seeing the following query generated:

SELECT (FLOOR((COUNT(*) / 1105908.0) * 100)) AS "percentage",
       "irc_message"."nickname", COUNT("irc_message"."id") AS "message_count"
FROM "irc_message"
WHERE "irc_message"."channel_id" = 3
GROUP BY "irc_message"."nickname",
         FLOOR((COUNT(*) / 1105908.0) * 100)
ORDER BY message_count DESC LIMIT 10

As you can see the expression from extra(select=) is sneaking into the GROUP BY clause. Tracking this down led to #10132 which seems to introduce this new behavior (even though I don't know if things worked before [9838]).

Change History (3)

comment:1 by Brian Rosner, 16 years ago

After thinking about this a bit more it does appear to be doing the most correct thing. My specific use-case was able to do done via a custom aggregate. I'll leave open just in-case there was something overlooked.

comment:2 by Russell Keith-Magee, 16 years ago

Resolution: invalid
Status: newclosed

Your thinking is correct. Any extra(select=) columns are assumed to be non-aggregates, and are therefore included in the GROUP BY; it is assumed that if you need an aggregate in the extra, you can use annotate() and a custom aggregate.

comment:3 by Jacob, 13 years ago

milestone: 1.1

Milestone 1.1 deleted

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