#2259: PK Change creates new object instead of update
-------------------------------------+-------------------------------------
Reporter: ed@… | Owner: nobody
Type: Bug | Status: reopened
Milestone: | Component: contrib.admin
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Design | Has patch: 1
decision needed | Needs tests: 0
Needs documentation: 1 | Easy pickings: 0
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* cc: anssi.kaariainen@… (added)
* has_patch: 0 => 1
* ui_ux: => 0
* easy: => 0
Comment:
I know this is unlikely to get accepted because of backwards compatibility
reasons, but here goes anyways.
The attached patch implements a version of save_base that automatically
does the following things when the PK has been changed since last loaded
from the DB:
- If there is already a row with the new pk and force_update is not set,
it errors out.
- If there is already a row with the new pk and force_update is set it
updates the row with the new pk and leaves the old pk alone.
- If there is no row with new or old pk, it inserts a new row.
- If pk has not been changed, everything works as always.
- The only way (without resorting to undocumented features) to get a pk
update is to use a model that comes from DB. save() counts as come from
DB.
The above tries to be intelligent and do the right thing. The current code
also tries to be intelligent and do the right thing, but IMHO it fails
when the PK has been changed. What it does is not what one would expect.
On the other hand the specification of what save() does is very clear. I
think the problem here is that we have the single method save() which
should always do the right thing. With changing PKs it is not entirely
clear what the right thing is.
Other possible ways to solve this ticket is to make a new method or to add
additional optional parameters to save. In this case I think .save()
without force_update or force_insert should raise an Error if the primary
key has changed.
The biggest backwards incompatibility is this (present also in Django test
suite):
{{{
Lets save some objects:
obj = T()
t.pk = 123
t.name = 'foob'
t.save()
t.pk = 456
t.name = 'faab'
t.save()
# Or better yet, do that in loop...
}}}
Old code would have saved two objects, the new code updates the first
saved. So as is this patch has no change to get in. On the other hand,
if/when composite primary keys are going to be included in Django, the
current save() is going to cause a lot of wondering...
--
Ticket URL: <https://code.djangoproject.com/ticket/2259#comment:21>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
