#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')
Another thing that fails is this:
Category.objects.all().annotate(tiles_count=Count("tiles")).filter(tiles_count__gt=0).update(used=True)
Anyway the problem is due to the fact that it tries to use Category.id although it seems to be out of scope.
The problem seem to be with the SQLUpdateCompiler or somewhere in the Query object, I searched but I know very little about this field.
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 #25171 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?