#22967: Model._do_update not always consistent with the precise meaning of
returned
value
----------------------------------------------+--------------------
Reporter: knaperek | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
''This is a bit obscure corner-case bug which might seem unimportant, but
the solution is easy and actually only makes the code simpler (removes one
nested if) and more consistent.''
When {{{select_on_save}}} is set, {{{Model._do_update()}}} ignores the
result of {{{Model._update()}}} if it learnt that the object exists '''in
a previous DB call'''. That last thing is crucial, as the object could
have already been deleted by concurrent worker just in-between these two
ORM calls ({{{exists}}} and {{{_update}}}). In such case, the
{{{_update()}}} call will fail, resulting in nothing being actually
updated. However, {{{_do_update()}}} method will currently return
'''True''' regardless of {{{_update()}}} result, which is inconsistent
with the common (and expected) behavior, and also in contrast with the
method's docs (saying: ''If the model was updated (in the sense that an
update query was done and a matching row was found from the DB) the method
will return True.'')
As a consequence, object will not be saved into the database, because
{{{_save_table()}}} won't call {{{_do_insert()}}}, being misled by
{{{_do_update()}}}. Now you may oppose that this does not really matter
that much, since it has been caused by concurrent modifications of the
same data, which is considered non-deterministic anyway (so noone has the
right to complain that the object is not there at the end). However,
here's one example that vouches for this bug to be fixed: if you happen to
have a trigger in your DB counting the number of times an object was
changed (updated/inserted), you'd be missing one (which is no longer
justifiable by concurrent save() and delete() operations).
As it sometimes happens, it is IMHO impossible to create a unit test for
this bug, as it is too low level thing, exploitable only in a concurrent
environment, and with some (bad) luck. I only found the bug when studying
the code. So that's why I tried to provide a solid explanation, believing
the fix I'm about to make should be obvious and mergable without worries.
--
Ticket URL: <https://code.djangoproject.com/ticket/22967>
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/051.53da3887a47cc25bb20e2979f1949b7b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
