PK Change creates new object instead of update

This seems to be a problem with the DB layer too since it doesn't work in the shell either. When trying to modify the primary key, it inserts a new record instead of updating the existing record.

>>> from myproject.config.models import Group
>>> Group.objects.all()
[]
>>> g = Group(name="Test")
>>> g.save()
>>> Group.objects.all()
[<Group: Test>]
>>> g = Group.objects.get(name="Test")
>>> g.name = "TestX"
>>> g.save()
>>> Group.objects.all()
[<Group: Test>, <Group: TestX>]

Someone should write some tests up for this.

This bug causes that it is not possible to change primary key value of an object. It has nothing specific to Admin interface and also the bug appears for integers primary keys as well as for non-integer primary keys.

I've added a patch file that contains tests for this issue.

The problem is with django.db.models.base.Model class. If we have object that has primary key value set eg.:

>>> g = Group(name="Test")
>>> g.save()
>>> print g.id
1

then if we change id like:

g.id = 32 

then django.db.models.base.Model.save() method will use new(!) id value while checking for object existence in database. Of course if this object doesn't exists it will be created (INSERT statement) what is not an expected behaviour here (there should be an UPDATE I think). The question is what should happen if pk is set to another existing value.

This is because pk_val = self._get_pk_val() always returns current value of the object attibute and this is wrong for PK check. Possible solution should store somewhere original PK value and after successful model save (after executing cursor.commit()) should update this original value.

I wondered about using post_save signal for this but in case of 'managed' transaction it is wrong I think as setting original pk value should be bound to sucessful RDBMS commit only.

Changes at this area seems to have huge impact on whole framework so I think design decision is needed here.

I'm -1 on this -- for better or worse, an instance's PK is what uniquely identifies the object.

-1. Let's just fix the docs and set editable = false on PKs. If someone absolutely, positively wants to update his PKs he can write custom SQL.

This bug still exists. Whether we allow updating PK's or not, the admin still shows them as editable, and it fails to update it. As PhiR says, we need to at least set them editable=False.

BTW, I'm +1 on allowing them to be edited.

The bug is there, but maybe the thing is *to not allow pk changes*. Anyway what happens right now is that Django will insert a new item with the new pk, instead of updating the old one.

I was bitten by this a while go in a so called "real world application". The case was a legacy product database with tens of thousands of products which integrated with a 3rd party solution that used the PK:s in the API. The workflow of adding a product in the database was almost always in two parts because the needed PK came from the manufacturer; first they put the product in with basic info and a dummy PK. Then, when the actual product (and it's PK) arrived from manufacturer the PK was changed and all was fine.

We couldn't get Django Admin to cope with this so we wrote our own admin (no big deal). But yes, this is a bug and it should be fixed (and also documented, especially if the fix is "you cannot change PK:s with Django Admin").

Replying to Uninen:

We couldn't get Django Admin to cope with this so we wrote our own admin (no big deal). But yes, this is a bug and it should be fixed (and also documented, especially if the fix is "you cannot change PK:s with Django Admin").

Not exactly, you can't do that out of admin also. Django will INSERT a new object instead of updating the old one, as before UPDATE it does SELECT ... WHERE pk = <new pk>. So your custom admin maybe inserting new rows (if you're running trunk).

If you can add an autoincrement field to the database (ignored by the legacy app) your life will be much easier.

This issue has popped up on too many projects now, usually in the context of admin.

Either we allow custom PK value changes or not.

I propose we do, right from the ORM level up; and UPDATE instead of INSERT if an _original_pk was present but is different from the new PK at time of saving. Problem solved, and Django can then claim full support of custom single-field primary keys.

If not, then I guess that's acceptable too; but at the very least it needs to be made clear in the documentation, and enforced in newforms-admin.

Please do not change the milestone (particularly since you've already decided to reopen a ticket that was wontfixed without a django-dev discussion). We've already decided on the 1.0 alpha and beta features and this is an enhancement request.

This can quite comfortably be put off until after 1.0. I'm probably +1 on finding a way for primary keys to be updated, but the current default behaviour should probably remain. Having a paramater to control this won't be the end of the world, since it's not the most common use-case on the planet (but I agree that when you need to do it, you really, really need it).

Marking as post-1.0 for now.

Milestone post-1.0 deleted

I think this issue is actually illustrating a greater flaw in the overall transactional structure the Django ORM implements.

Right now a save transaction works like this:

obj.save

transaction> SELECT obj

if exists: UPDATE else: INSERT <transaction

There are several issues with this. In a write heave database this will likely lead to serious performance issues. If an object gets deleted, there is the chance in a race-condition that it will be re-inserted... and obviously you can't change the primary key, because the whole transaction works based off of whether an object exists with the same PK.

How it should work:

obj = MyModel?.objects.get(id = 1) # there is a private variable set with the original PK obj.field = 1 obj.save() # because there is the original PK set, it does an update

So how it works here is based off of private variables. Going beyond having a PK that is private, you could also have the rest of the fields have non-mutable private variables to generate cleaner, shorter SQL statements ... though I'm not sure that would have any real benefit.

There should only be inserts on new objects...

obj = MyModel?()

... and there is absolutely no reason to do a SELECT at the start of the transactional block. In write heavy applications, this will be horrible for performance.