Opened 15 years ago

Closed 5 years ago

Last modified 5 years ago

#9982 closed Bug (fixed)

Inconsistent behavior on model save depending on whether OneToOneField is a primary key

Reported by: sean@… Owned by:
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: OneToOneField primary_key IntegrityError
Cc: Can Sarıgöl Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,
When using a OneToOneField to make one model extend another, I noticed a behavioral inconsistency depending on whether the related model's OneToOneField was also the primary key or not.

The inconsistency is to do with when save() can be called on model instances.
Consider the following example models:

class BaseModel(models.Model):
    pass

class ExtendedModel(models.Model):
    link = models.OneToOneField(BaseModel)

It is not possible to first instantiate these models and later on save them - an IntegrityError would be raised when saving the ExtendedModel. E.g.,

o1 = BaseModel()
o2 = ExtendedModel(link=o1)
o1.save()
o2.save()

when o2 is saved it will raise an IntegrityError because it doesn't have a value for the primary key of the model it is related to. I think o1's primary key is copied when o2 is initialized, and is None at this point because o1 isn't saved yet.

However, if the definition of ExtendedModel is changed so that the OneToOneField is also the primary key, it now becomes possible to save the model instances in this order. To test this I changed the model definition as follows:

class ExtendedModel(models.Model):
    link = models.OneToOneField(BaseModel, primary_key=True)

When using this model definition it seems that o1's primary key is copied when o2 is saved, as opposed to when o2 is initialized. This inconsistency caused a lot of head scratching here.

Thanks!
Sean

Change History (11)

comment:1 Changed 15 years ago by Malcolm Tredinnick

Owner: changed from nobody to Malcolm Tredinnick
Status: newassigned

The second case shouldn't work either. I'll have to check whether it's really doing something sensible there (as opposed to just not raising an error). It may be because of something in place for model inheritance that it's working; I seem to recall we added an admin-related workaround that was a bit of a hack. In which case, it'll just need documentation.

Django cannot read your mind and does not automatically save things. So you do need to save something before using it as a reference. The first case is consistent and you should always save before using something as a related instance.

Leaving open until I understand and document (at a minimum) what's happening.

comment:2 Changed 15 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:3 Changed 15 years ago by Jacob

milestone: 1.1
Triage Stage: UnreviewedAccepted

comment:4 Changed 15 years ago by Jacob

milestone: 1.11.2

Pushing to 1.2: this is an inconsistancy, not an outright bug.

comment:5 Changed 14 years ago by Malcolm Tredinnick

milestone: 1.2
Owner: Malcolm Tredinnick deleted
Status: assignednew

Moving off the 1.2 milestone. As noted in the first comment, this should be raising an error in the case that appears to work, in the best case. So it's not a case of something not working that should.

comment:6 Changed 13 years ago by Chris Beaven

Severity: Normal
Type: Bug

comment:7 Changed 12 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:8 Changed 12 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:9 Changed 5 years ago by Can Sarıgöl

Cc: Can Sarıgöl added

Hi, I think this bug is fixed by that PR

comment:10 Changed 5 years ago by Mariusz Felisiak

Resolution: fixed
Status: newclosed
Version: 1.0master

comment:11 Changed 5 years ago by GitHub <noreply@…>

In 85195dd2:

Refs #9982 -- Added test for saving OneToOneField field after saving related object.

Fixed in 519016e5f25d7c0a040015724f9920581551cab0.

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