Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#28228 closed Bug (wontfix)

changing a primary key and saving results in a new object rather than renaming the existing object.

Reported by: Chris Withers Owned by: nobody
Component: Database layer (models, ORM) Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

See https://github.com/encode/django-rest-framework/issues/5158.

Incorrect behaviour in https://github.com/encode/django-rest-framework/issues/5158#issuecomment-302679922.

Expected behaviour:

MyModel.objects.all()
# <QuerySet [<MyModel: test>, <MyModel: test1>]>
instance = MyModel.objects.all()[0]
instance.name
# 'test'
instance.name = 'other test'
instance.save()
MyModel.objects.all()
# <QuerySet [<MyModel: test1>, <MyModel: other test>]>

Change History (7)

comment:1 by Tom Forbes, 7 years ago

I think this is expected: https://docs.djangoproject.com/en/1.11/topics/db/models/

The primary key field is read-only. If you change the value of the primary key on an existing object and then save it, a new object will be created alongside the old one. For example...

Version 0, edited 7 years ago by Tom Forbes (next)

comment:2 by Chris Withers, 7 years ago

Documenting buggy behaviour doesn't make it any less buggy. How are you supposed to handle renames?
(Normal relational database behaviour is that change of the primary key results in a change to the primary key, with cascading if so configured, or an IntegrityError if not and there are foreign keys referring to this).

comment:3 by Mariusz Felisiak, 7 years ago

Resolution: wontfix
Status: newclosed

I wouldn't say it's a buggy behavior. It's well documented expected behavior. See related tickets #17332, #14071.

comment:4 by Tim Graham, 7 years ago

I found an old proposal to deprecate the current behavior. Perhaps that conversation could be resumed -- I don't think there's been any other recent discussion about it on the mailing list.

comment:5 by Marten Kenbeek, 7 years ago

You can change the primary key using filter() and update():

MyModel.objects.filter(name='test1').update(name='other test')

comment:6 by Chris Withers, 7 years ago

@marten - I can, but I suspect that's exactly the kind of hack that the DRF guys are reluctant to implement...

comment:7 by Tom Christie, 7 years ago

I think the "wontfix" resolution against the ticket as stated makes sense.

However, there's a subset of this behavior that isn't documented, which is the behavior when a primary key is renamed to the value of an already existing pk.

Given this...

If you change the value of the primary key on an existing object and then save it, a new object will be created alongside the old one

I would expect to see the same behavior as if attempting to create a new object with an already used pk. This *ought* to result in an integrity error, given the uniqueness constraint.
The actual behavior currently is that the existing object will be overwritten.

That doesn't seem like necessarily the most intuitive outcome, and it's easy to see how it could introduce unexpected bugs or more-permissive-than-intended behavior.

I haven't seen this specific subset of the issue isolated in any of the related tickets/discussions, so I think there might be something worth considering there.

Note: See TracTickets for help on using tickets.
Back to Top