#36489: OneToOneField + concurrent get_or_create results in wrong object in
field
cache
-------------------------------------+-------------------------------------
Reporter: Brian Atkinson | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
> From my perspective as a user, I might prefer an additional read to
populate that relation over that inequality not holding. At least I think
that would be my preference.
Proper cache invalidation would effectively be the less risky option but
it's far from trivial do to correctly. We can't systematically clear the
cache as it might discard explicit relationship retrieval through
`select_related` as alluded above.
> Is there an assumption that the following should be true?
> `assert child is parent.child`
No, in particular when `created is False` like it's the case here.
Think of it this way, `get_or_create` is meant to be a concurrency safe
hybrid between `get` and `create` so users don't have to re-implement it
in an unsafe way every time the ''upsert'' pattern is needed.
In the case of `get` the `child is parent.child` assumption will never
hold true, that is
{{{#!python
>>> child = Child.objects.get(parent=parent)
>>> child is parent.child
False
}}}
as there is no session identity mapper. On the hand, because of
implementation details, the assumption will hold true for `create`
{{{#!python
>>> child = Child.objects.create(parent=parent)
>>> child is parent.child
True
}}}
which brings the question of what exactly should be done in cases where
`get_or_create` falls back to `get` (`created is False`) given it's meant
to be an hybrid. Is it better to keep things as they are today and avoid
introducing an exception when a to-one relationship is provided and the
object already exists or is it better to align it to-many related managers
which offer a way to tap-in this per-queryset identity mapper logic that
to-one relationship cannot take advantage of because attribute access
doesn't yield a manager
{{{#!python
>>> author.books.create(title="2001: A Space Odyssey")
>>> book, created = author.books.get_or_create(title="2001: A Space
Odyssey")
>>> created # get() was used.
False
>>> book.author is author
True
}}}
I'll have to ponder on that one a bit because while I can see the appeal
it introduces significant complexity with high risk of subtle breakages.
If we are fixing this I'm of opinion that we should make it work for
`get_or_create` under all circumstances where `get` is used and not only
on the `IntegrityError` fallback as that would be even more confusing than
it is today.
--
Ticket URL: <https://code.djangoproject.com/ticket/36489#comment:6>
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 unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To view this discussion visit
https://groups.google.com/d/msgid/django-updates/01070197c89bc291-01fab4a8-a444-4761-9e2d-21d669718e37-000000%40eu-central-1.amazonses.com.
