#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 , 9 years ago
Description: | modified (diff) |
---|---|
Keywords: | annotate count added; filter removed |
follow-up: 3 comment:2 by , 9 years ago
comment:3 by , 9 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 , 9 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 , 9 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 , 9 years ago
This would be legit
categories = Category.objects.annotate(Count('tiles')) # some code categories.update(name='foo')
comment:7 by , 9 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 , 9 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 , 9 years ago
Is this a duplicate of or related to #19513? It describes a failure of update()
after annotate(val=Sum(...))
.
comment:10 by , 9 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?