[Django] #34433: OneToOneField can only be saved one way
Django
-------------------------------------+-------------------------------------
Reporter: Alexis | Owner: nobody
Lesieur |
Type: Bug | Status: new
Component: Database | Version: 4.1
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Hi!
I encountered this unexpected (to me) behavior for work, and I have been
able to replicate on a bare django app (albeit with slightly different
symptoms).
The TLDR is that is model A has a OneToOneField to model B. The field had
to be saved from the instance of model A, and that's not only not
documented anywhere I could find, but counter-intuitive, and contradicts
how other fields like ForeignKeys work.
**Setup:**
{{{
❯ python --version
Python 3.11.2
❯ pip freeze | grep -i django
Django==4.1.7
❯ django-admin startproject mysite
❯ cd mysyte/
❯ django-admin startapp myapp
❯ vim myapp/models.py
# partially re-using your example from
https://docs.djangoproject.com/en/4.1/topics/db/examples/one_to_one/
```
from django.db import models
class Place(models.Model):
name = models.CharField(max_length=50)
address = models.CharField(max_length=80)
def __str__(self):
return "%s the place" % self.name
class Restaurant(models.Model):
place = models.OneToOneField(
Place,
on_delete=models.CASCADE,
)
serves_hot_dogs = models.BooleanField(default=False)
serves_pizza = models.BooleanField(default=False)
def __str__(self):
return "%s the restaurant" % self.place.name
```
❯ vim mysite/settings.py
[...]
INSTALLED_APPS = [
'myapp.apps.MyappConfig',
[...]
❯ python manage.py makemigrations
❯ python manage.py migrate
}}}
Creating the initial objects:
{{{
❯ python manage.py shell
❯ from myapp.models import Place
❯ from myapp.models import Restaurant
❯ p1 = Place(name="1st place", address="1st address")
❯ p2 = Place(name="2nd place", address="2nd address")
❯ r1 = Restaurant(place=p1)
❯ r2 = Restaurant(place=p2)
❯ p1.save()
❯ p2.save()
❯ r1.save()
❯ r2.save()
❯ p3 = Place(name="3rd place", address="3rd address")
❯ p3.save()
}}}
This should give us a two restaurants with their respective places, and an
additional place we can use to play.
First, what works:
{{{
❯ r1.place = p3
❯ r1.save()
❯ Restaurant.objects.get(id=1).place
<Place: 3rd place the place>
❯ p3.restaurant
<Restaurant: 3rd place the restaurant>
❯ Place.objects.get(id=1).restaurant
[...]
RelatedObjectDoesNotExist: Place has no restaurant.
}}}
This is all expected. `r1` now uses `p3`, which means that `p1` has no
restaurant assigned.
Now I would expect, to be able to do the other way. Assign a new
restaurant to a place, save, and be all good.
However that doesn't work.
First using plain `.save()` which fails silently:
{{{
❯ p1 = Place.objects.get(id=1)
❯ p1.restaurant = r1
❯ p1.save()
❯ Restaurant.objects.get(id=1).place
<Place: 3rd place the place> # this should be p1
}}}
And when explicitly asking to save the field:
{{{
❯ p1.save(update_fields=["restaurant"])
❯ ValueError: The following fields do not exist in this model, are m2m
fields, or are non-concrete fields: restaurant
}}}
NB: on my use case for work (django 3.2.18) I was also getting the
following error:
{{{
UniqueViolation: duplicate key value violates unique constraint
"response_timelineevent_pkey"
DETAIL: Key (id)=(91) already exists.
}}}
I'm not sure why it's different, but it doesn't work either way.
This is problematic for a few reasons IMO:
- Unless I missed it, the docs really don't advertise this limitation.
- `.save()` "fails" silently, there is no way to know that the change
didn't take, especially when this happens:
{{{
❯ p1 = Place(name="1st place", address="1st address")
❯ p2 = Place(name="2nd place", address="2nd address")
❯ p3 = Place(name="3rd place", address="3rd address")
❯ p1.save()
❯ p2.save()
❯ p3.save()
❯ r1 = Restaurant(place=p1)
❯ r1.save()
❯ r2 = Restaurant(place=p2)
❯ r2.save()
❯ r1.place
<Place: 1st place the place>
❯ p3.restaurant = r1
❯ r1.place
<Place: 3rd place the place>
❯ p3.save()
❯ Restaurant.objects.get(id=1).place
<Place: 1st place the place>
}}}
which leads to thinking the change is working and affecting both objects,
when it's not.
It's also problematic as foreigh keys work this way: (from my work
example)
{{{
❯ me = ExternalUser.objects.get(id=1)
❯ other = ExternalUser.objects.get(id=2)
❯ p = PinnedMessage.objects.get(id=11)
❯ p.author
<ExternalUser: first.last (slack)> # i.e. `me`
❯ [p.id for p in me.authored_pinnedmessage.all()]
[1, 3, 5, 11]
❯ p.author = other
❯ p.save()
❯ [p.id for p in
ExternalUser.objects.get(id=1).authored_pinnedmessage.all()]
[1, 3, 5]
❯ me.authored_pinnedmessage.add(p)
❯ me.save()
❯ PinnedMessage.objects.get(id=11).author
<ExternalUser: first.last (slack)>
}}}
Hopefully this is all enough explanation / details.
Let me know if you need anything else from me!
Thank you for your help.
--
Ticket URL: <https://code.djangoproject.com/ticket/34433>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
❯ django-admin startproject mysite
❯ vim mysite/settings.py
❯ p.author = other
❯ p.save()
❯ me.authored_pinnedmessage.add(p)
❯ me.save()
❯ PinnedMessage.objects.get(id=11).author
<ExternalUser: first.last (slack)>
}}}
[EDIT] This is also counterintuitive because the documentation for
`OneToOneField` explicitely states:
> A one-to-one relationship. Conceptually, this is similar to a ForeignKey
with unique=True, but the “reverse” side of the relation will directly
return a single object.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34433#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Alexis Lesieur | Owner: nobody
Component: Database layer | Version: 4.1
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* type: Bug => New feature
* resolution: => duplicate
Comment:
Duplicate of #18638. You can comment in the original ticket. If you don't
agree with the decision made in #18638 please
[https://docs.djangoproject.com/en/stable/internals/contributing
/triaging-tickets/#closing-tickets follow triaging guidelines with
regards to wontfix tickets].
--
Ticket URL: <https://code.djangoproject.com/ticket/34433#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Alexis Lesieur | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> It's also problematic as foreigh keys work this way: (from my work
example)
That's not true, on foreign keys you also cannot assign to a **reverse**
side.
> `.save()` "fails" silently, there is no way to know that the change
didn't take, especially when this happens:
`save()` doesn't fail silently. By assigning an object to the
`.restaurant` you override a lazy reverse side of `OneToOneField` to a
"static" instance so it's not recognize as a relationship anymore.
--
Ticket URL: <https://code.djangoproject.com/ticket/34433#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Alexis Lesieur | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alexis Lesieur):
First off apologies for missing the older ticket. I searched for similar
problems but...
Replying to [comment:3 Mariusz Felisiak]:
> > It's also problematic as foreigh keys work this way: (from my work
example)
>
> That's not true, on foreign keys you also cannot assign to a **reverse**
side.
You cannot "assign" as I may not just be able to do
`me.authored_pinnedmessage = ...` (to reference my earlier example) but I
can definitely do:
{{{
❯ PinnedMessage.objects.get(id=1).author
<ExternalUser: other.user (slack)>
❯ me = ExternalUser.objects.get(id=1)
❯ me.authored_pinnedmessages.add(p)
❯ me.save()
❯ PinnedMessage.objects.get(id=1).author
<ExternalUser: first.last (slack)>
}}}
And this works. So I can definitely use the reverse relationship.
Replying to [comment:3 Mariusz Felisiak]:
> > `.save()` "fails" silently, there is no way to know that the change
didn't take, especially when this happens:
>
> `save()` doesn't fail silently. By assigning an object to the
`.restaurant` you override a lazy reverse side of `OneToOneField` to a
"static" instance so it's not recognize as a relationship anymore.
I wholeheartedly disagree. Not only does it recognize the relationship
still as this proves:
{{{
❯ p1 = Place(name="1st place", address="1st address")
❯ p1.save()
❯ r1 = Restaurant(place=p1)
❯ r1.save()
❯ r2 = Restaurant(place=p2)
❯ r2.save()
❯ p1.restaurant = r2
❯ r2.place
<Place: 1st place the place>
}}}
It is clearly reflected. The relationship is definitely recognized.
It is just being ignored on save.
When as we showed it's being honored for ForeignKeys. Which the
documentation claims behaves the same.
I understand that from a pure implementation pov you're telling me that
doing `model.related_set.add(...)` is different than doing
`place.restaurant = new_place` here.
But that's really besides the point. From a user POV, the behavior is
highly unexpected, and really the main problem IMO (assuming you do keep
the current behavior) the documentation is really not addressing this
issue in any way.
In addition to the "OneToOneFields are just ForeignKeys with unique=True"
the documentation on how to use them clearly says:
> Set the place using assignment notation. Because place is the primary
key on Restaurant, the save will create a new restaurant:
{{{
>>> r.place = p2
>>> r.save()
>>> p2.restaurant
<Restaurant: Ace Hardware the restaurant>
>>> r.place
<Place: Ace Hardware the place>
}}}
>Set the place back again, using assignment in the reverse direction:
{{{
>>> p1.restaurant = r
>>> p1.restaurant
<Restaurant: Demon Dogs the restaurant>
}}}
Without addressing anywhere that you can't do p1.save() here. Which is
very counterintuitive. It is extremely expected that if I do
`p1.restaurant = r`, to apply/save that change, I should be doing
`p1.save()`.
If we're saying this is not applicable, please do call it out in the
documentation :-)
--
Ticket URL: <https://code.djangoproject.com/ticket/34433#comment:4>