#25171 closed Bug (duplicate)
Can't update queryset after Count annotation
| Reported by: | Fraser Harris | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 1.6 |
| Severity: | Normal | Keywords: | queryset, update, annotate, count |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
A Queryset that has been annotated with a Count of a ManyToMany field can not be update'd.
Example:
>>> Category.objects.annotate(Count('tiles')).update(name='foo')
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/opt/secondfunnel/venv/local/lib/python2.7/site-packages/django/db/models/query.py", line 490, in update
rows = query.get_compiler(self.db).execute_sql(None)
File "/opt/secondfunnel/venv/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 976, in execute_sql
cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
File "/opt/secondfunnel/venv/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 782, in execute_sql
cursor.execute(sql, params)
File "/opt/secondfunnel/venv/local/lib/python2.7/site-packages/django/db/backends/util.py", line 69, in execute
return super(CursorDebugWrapper, self).execute(sql, params)
File "/opt/secondfunnel/venv/local/lib/python2.7/site-packages/django/db/backends/util.py", line 53, in execute
return self.cursor.execute(sql, params)
File "/opt/secondfunnel/venv/local/lib/python2.7/site-packages/django/db/utils.py", line 99, in __exit__
six.reraise(dj_exc_type, dj_exc_value, traceback)
File "/opt/secondfunnel/venv/local/lib/python2.7/site-packages/django/db/backends/util.py", line 53, in execute
return self.cursor.execute(sql, params)
ProgrammingError: subquery has too many columns
LINE 1: ..." SET "name" = 'foo' WHERE "assets_category"."id" IN (SELECT...
^
Models:
class Category(BaseModel):
tiles = models.ManyToManyField(Tile, related_name='categories')
name = models.CharField(max_length=255)
class Tile(Model):
pass
SQL Query from django.db.connections.queries:
UPDATE "assets_category" SET "name" = \'foo\' WHERE "assets_category"."id" IN (SELECT U0."id", COUNT(U2."tile_id") AS "tiles__count" FROM "assets_category" U0 LEFT OUTER JOIN "assets_category_tiles" U2 ON ( U0."id" = U2."category_id" ) GROUP BY U0."id", U0."name")
Change History (10)
comment:1 by , 10 years ago
| Description: | modified (diff) |
|---|---|
| Keywords: | annotate count added; filter removed |
follow-up: 3 comment:2 by , 10 years ago
comment:3 by , 10 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
Replying to timgraham:
I guess trying to update after annotations or aggregations probably isn't designed to work. Should we throw a friendlier error message?
Well, certainly not after a terminal aggregation. But I don't see any a priori reason to assume that if you have an annotated QuerySet you shouldn't be able to update it. ISTM this should be accepted as a bug, and we should only resort to "friendlier error message" if somebody looks at it closely and reports more authoritatively that it's not feasible to support.
follow-up: 5 comment:4 by , 10 years ago
For the query given in the report:
Category.objects.annotate(Count('tiles')).update(name='foo')
I would like to see a warning (if not an error) because the annotation is "dead code" and probably a bug.
However, I'd expect this to work (assuming an additional numeric field "score" on the Category model)
Category.objects.annotate(n_tiles=Count('tiles')).update(score=F('score')+F('n_tiles'))
comment:5 by , 10 years ago
Replying to shaib:
For the query given in the report:
Category.objects.annotate(Count('tiles')).update(name='foo')I would like to see a warning (if not an error) because the annotation is "dead code" and probably a bug.
Sure, but unless that falls out naturally in an implementation that supports the useful cases, I think it's firmly in nice-to-have territory. An implementation that let this pass silently wouldn't bother or surprise me at all.
comment:6 by , 10 years ago
This would be legit
categories = Category.objects.annotate(Count('tiles'))
# some code
categories.update(name='foo')
comment:7 by , 10 years ago
The query that would work in the above situation (aggregate, filter, update) would be roughly:
update category
set used=true
where id in (
select X.id
from (select id, COUNT(tiles_id) tile_count from category group by id) X
where X.tile_count >= 0
)
-- OR preferably?
update category
set used=true
where id in (
select id from category group by id
having COUNT(tiles_id) > 0
)
Which I think can be generated with roughly the following ORM code (not tested):
inner = Category.objects.all().annotate(tiles_count=Count("tiles")).filter(tiles_count__gt=0)
# maybe inner = inner.values('pk') ?
Category.objects.filter(pk__in=inner).update(used=True)
It might be possible to get SQLUpdateCompiler to detect annotations, and then create a new queryset, passing the existing queryset as an argument to pk__in=previous. Or teach it to drop anything from the subquerys annotations_select but ensure it stills GROUPs and HAVINGs.
comment:8 by , 10 years ago
If you take out the annotation part then you wouldn't be able to use annotated values in filters nor in F functions.
comment:9 by , 10 years ago
Is this a duplicate of or related to #19513? It describes a failure of update() after annotate(val=Sum(...)).
comment:10 by , 10 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | new → closed |
Duplicate of #19513 as Tim pointed out.
I guess trying to update after annotations or aggregations probably isn't designed to work. Should we throw a friendlier error message?