#19513: update() after annotate(val=Sum(...)) crashes on PostgreSQL & Oracle
-------------------------------------+-------------------------------------
Reporter: mengzhuo1203@… | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.4
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by dsanders11):
Has there been any recent progress on this issue? It really limits adding
annotations in a Manager because those annotations may prevent updates.
I'm seeing this with sqlite3 as well, so the original (3 year old)
description is not accurate in that regard. I believe MySQL is OK for the
reason in [comment:1 #1], it appears to do a SELECT first and then update
only those IDs. The example code [comment:2 akaariai gave] still
replicates the issue on sqlite3 with the slight tweak that 'get_query_set'
is now 'get_queryset'.
I have
[https://github.com/dsanders11/django/commit/4431905249ffb8684127ea809688c80f4a64f051
a fix] that I believe nicely resolves the issue, but I don't know nearly
enough about query and SQL generating codebase to know if it's missing
something obvious or will cause other potential problems.
The fix is fairly simple I simply clear annotations (unsure if the
`set_annotation_mask` call is necessary) on the query after it is cloned
and the update values are added in QuerySet.update. That resolved 90% of
the issue. On #25171
[https://code.djangoproject.com/ticket/25171#comment:8 zauddelig made a
comment] that this approach of clearing annotations would mess with
filters and and F functions using the annotations. At least with filters
on sqlite3 this is not a problem, as the filters are applied to the query
before the update occurs and simply inserts the resolved annotation into
the WHERE clause. Aggregations are already not allowed in the update so
really it's only situations where you have an annotation like
annotate(range=F('max') - F('min')).update(foo=F('range')). I fixed this
by resolving the update values in UpdateQuery. add_update_fields so that
they're resolved before stripping the annotations from the UPDATE query.
I believe this fix to be 'safe' in the sense that annotations create new
names and aren't allowed to shadow a field on the model so any potential
problem from clearing the annotations should appear as a name failing to
resolve in a query because the annotation was removed. I don't believe it
could cause any subtle mischief or corruption.
Is there any risk in resolving expressions at the time they're added to
the update query as an update field?
BTW, zauddelig's comment actually points out an edge case in the MySQL
generated code (and for a different reason sqlite3, I tested both) where
if you try to use an annotation which spans tables in an update,
everything will resolve and attempt to execute but the SQL will fail
because for MySQL the query is broken into a SELECT and then an UPDATE,
but the necessary info is not available in the UPDATE.
For example:
{{{
class Bar(models.Model):
name = models.CharField(max_length=32)
class Foo(models.Model):
related_bar = models.ForeignKey(Bar)
bar_name = models.CharField(max_length=32)
----------------------
Foo.objects.annotate(bar_name=F('related_bar__name')).update(name=F('bar_name'))
------------------------
SELECT `test_foo`.`id`, `test_bar`.`name` AS `bar_name` FROM `test_foo`
LEFT OUTER JOIN `test_bar` ON ( `test_foo`.`related_bar` = `test_bar`.`id`
);
UPDATE `test_foo` SET `bar_name` = `test_bar`.`name` WHERE `test_foo`.`id`
IN (1);
}}}
The generated SQL for sqlite3 is similar with the SELECT inside the WHERE
clause. Seems like if this case is allowed (which is a narrow use case
since aggregates aren't allowed in the update at the moment) it would need
to add the JOIN to the update query for everything to work as expected.
Instead it fails currently during the SQL execution. Note, I only tested
this on Django 1.8.7 but I haven't seen any recent changes in the related
code to suggest that a similar result wouldn't happen on master.
--
Ticket URL: <https://code.djangoproject.com/ticket/19513#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/080.b63dde2270b7a5078d90aba33eebd710%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
