Re: [Django] #34145: Explicit GROUPing by aggregate is not supported

5 views

Skip to first unread message

Django

unread,

Nov 9, 2022, 5:03:12 AM11/9/22

to django-...@googlegroups.com

#34145: Explicit GROUPing by aggregate is not supported
-------------------------------------+-------------------------------------
Reporter: Karolis Ryselis | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Karolis Ryselis):

The tricky part is when you try to add more fields to group by.
My actual Packing and PackingItem models have more fields, so what I am
actually trying to achieve is something like

{{{#!python
PackingItem.objects.filter(packing__in=my_packings,
amount__gt=0).annotate(tags=GroupConcat('packing__tags',
separator=',')).values('unit', 'good',
'tags').annotate(total_amount=Sum('amount'))
}}}

What happens inside is that unit and good (FK fields in my case) are added
to group by while tags is silently ignored and I get an actual queryset
grouped by part of the fields that I passed to values. I believe that this
is a case where an exception could thrown stating that this is not
supported rather than giving "best effort" result.

--
Ticket URL: <https://code.djangoproject.com/ticket/34145#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Nov 9, 2022, 10:15:17 AM11/9/22

to django-...@googlegroups.com

Comment (by Simon Charette):

Let's decorticate the queryset your building step by step (I'll drop the
filter as it's not significant)

{{{#!python
PackingItem.objects.annotate(tags=GroupConcat('packing__tags'))
}}}

Generates

{{{#!sql
SELECT packingitem.*, GROUP_CONCAT(packing_tags.tag_id)
FROM packingitem
LEFT JOIN packing ON (packing.id = packingitem.packing_id)
LEFT JOIN packing_tags ON (packing_tags.packing_id = packing.id)
GROUP BY packingitem.id
}}}

Then, which is still entirely valid and we can't error out on

{{{#!python
PackingItem.objects.annotate(
tags=GroupConcat('packing__tags')

).values('unit', 'good', 'tags')
}}}

{{{#!sql
SELECT packingitem.unit, packingitem.good,
GROUP_CONCAT(packing_tags.tag_id)
FROM packingitem
LEFT JOIN packing ON (packing.id = packingitem.packing_id)
LEFT JOIN packing_tags ON (packing_tags.packing_id = packing.id)
GROUP BY packingitem.unit, packingitem.good
}}}

Then you'd want

{{{#!python
PackingItem.objects.annotate(
tags=GroupConcat('packing__tags')

).values('unit', 'good', 'tags').annotate(
total_amount=Sum('amount')
)
}}}

To either error out. The thing here is that I can't see an easy way to
perform a deprecation; `values()` has been dual purposed for years
(selecting fields and defining grouping) and changing that now will be
highly backward incompatible.

If you want to give it a shot yourself and see how the test suite behaves
to propose a patch here are the two locations you should look into
1. `annotate` method which sets `query.group_by` when an annotation of an
aggregate is made
([https://github.com/django/django/blob/d6fbfea08d9bae4165df6a0cfbd4520b5e2242b2/django/db/models/query.py#L1660-L1665
source])
2. `set_values` method which further adjust the group by based on the
provided values
([https://github.com/django/django/blob/d6fbfea08d9bae4165df6a0cfbd4520b5e2242b2/django/db/models/sql/query.py#L2446-L2464
source)

My guess is that you'll want to adjust 1. to raise an exception `if
instance(clone.query.group_by, tuple) and any(expr.contains_aggregate for
expr in clone.query)`.