[Django] #31891: Inconsistent related field cache invalidation

[Django]

#31891: Inconsistent related field cache invalidation
-------------------------------------+-------------------------------------
Reporter: Joshua | Owner: nobody
Doncaster-Marsiglio |
Type: | Status: new
Uncategorized |
Component: | Version: 3.1
Uncategorized | Keywords:
Severity: Normal | orm,related,onetoone
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Repro:

{{{
from django.db import models

class AppUser(models.Model):
pass

class Profile(models.Model):
user = models.OneToOneField(
to=AppUser,
on_delete=models.PROTECT,
)

a = AppUser.objects.create()
b = AppUser.objects.create()
Profile.objects.create(user=a)

b.profile # Errors, since it has not been created yet

a.profile.user_id = b.id
a.profile.save()

# Works!
Profile.objects.get(user=b)
# <Profile: Profile object (1)>

b.profile # Errors, since it uses the cached value
}}}

I can fix it by using `a.profile.user = b` which does the cache
invalidation as expected. Or using `b.refresh_from_db()`
This is a contrived example but the real use-case was cloning a model
instance.

--
Ticket URL: <https://code.djangoproject.com/ticket/31891>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#31891: Inconsistent related field cache invalidation
-------------------------------------+-------------------------------------

Reporter: Joshua Doncaster- | Owner: nobody
Marsiglio |
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
orm,related,onetoone | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Joshua Doncaster-Marsiglio:

Old description:

> Repro:
>
> {{{
> from django.db import models
>

> class AppUser(models.Model):
> pass
>

> class Profile(models.Model):
> user = models.OneToOneField(
> to=AppUser,
> on_delete=models.PROTECT,
> )
>

> a = AppUser.objects.create()
> b = AppUser.objects.create()
> Profile.objects.create(user=a)
>
> b.profile # Errors, since it has not been created yet
>
> a.profile.user_id = b.id
> a.profile.save()
>
> # Works!
> Profile.objects.get(user=b)
> # <Profile: Profile object (1)>
>
> b.profile # Errors, since it uses the cached value
> }}}
>
> I can fix it by using `a.profile.user = b` which does the cache
> invalidation as expected. Or using `b.refresh_from_db()`
> This is a contrived example but the real use-case was cloning a model
> instance.

New description:

Repro:

{{{
from django.db import models

class AppUser(models.Model):
pass

class Profile(models.Model):
user = models.OneToOneField(
to=AppUser,
on_delete=models.PROTECT,
)

a = AppUser.objects.create()
b = AppUser.objects.create()
Profile.objects.create(user=a)

b.profile # Errors, since it has not been created yet

a.profile.user_id = b.id
a.profile.save()

# Works!
Profile.objects.get(user=b)
# <Profile: Profile object (1)>

b.profile # Errors, since it uses the cached value
}}}

I can fix it by using `a.profile.user = b` which does the cache
invalidation as expected. Or using `b.refresh_from_db()`
This is a contrived example but the real use-case was cloning a model

instance. And instead of erroring it just kept returning the cached value

--

--
Ticket URL: <https://code.djangoproject.com/ticket/31891#comment:1>

[Django]

#31891: Inconsistent related field cache invalidation
-------------------------------------+-------------------------------------

Reporter: Joshua Doncaster- | Owner: nobody
Marsiglio |
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
orm,related,onetoone | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Joshua Doncaster-Marsiglio:

Old description:

> Repro:
>
> {{{
> from django.db import models
>

> class AppUser(models.Model):
> pass
>

> class Profile(models.Model):
> user = models.OneToOneField(
> to=AppUser,
> on_delete=models.PROTECT,
> )
>

> a = AppUser.objects.create()
> b = AppUser.objects.create()
> Profile.objects.create(user=a)
>
> b.profile # Errors, since it has not been created yet
>
> a.profile.user_id = b.id
> a.profile.save()
>
> # Works!
> Profile.objects.get(user=b)
> # <Profile: Profile object (1)>
>
> b.profile # Errors, since it uses the cached value
> }}}
>
> I can fix it by using `a.profile.user = b` which does the cache
> invalidation as expected. Or using `b.refresh_from_db()`
> This is a contrived example but the real use-case was cloning a model

> instance. And instead of erroring it just kept returning the cached value

New description:

Repro:

{{{
from django.db import models

class AppUser(models.Model):
pass

class Profile(models.Model):
user = models.OneToOneField(
to=AppUser,
on_delete=models.PROTECT,
)

a = AppUser.objects.create()
b = AppUser.objects.create()
Profile.objects.create(user=a)

b.profile # Errors, since it has not been created yet

a.profile.user_id = b.id
a.profile.save()

# Works!
Profile.objects.get(user=b)
# <Profile: Profile object (1)>

b.profile # Errors, since it uses the cached value
}}}

I can fix it by using `a.profile.user = b` which does the cache

invalidation as expected. Or using `b.refresh_from_db()`. However I think
it makes sense to fix this within the ORM because it's a subtle bug that
can cause issues.

This is a contrived example but the real use-case was cloning a model

instance. And instead of erroring it just kept returning the cached value

--

--
Ticket URL: <https://code.djangoproject.com/ticket/31891#comment:2>

[Django]

#31891: Inconsistent related field cache invalidation
-------------------------------------+-------------------------------------

Reporter: Josh | Owner: nobody

Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
orm,related,onetoone | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Josh:

Old description:

> Repro:
>
> {{{
> from django.db import models
>

> class AppUser(models.Model):
> pass
>

> class Profile(models.Model):
> user = models.OneToOneField(
> to=AppUser,
> on_delete=models.PROTECT,
> )
>

> a = AppUser.objects.create()
> b = AppUser.objects.create()
> Profile.objects.create(user=a)
>
> b.profile # Errors, since it has not been created yet
>
> a.profile.user_id = b.id
> a.profile.save()
>
> # Works!
> Profile.objects.get(user=b)
> # <Profile: Profile object (1)>
>
> b.profile # Errors, since it uses the cached value
> }}}
>
> I can fix it by using `a.profile.user = b` which does the cache

> invalidation as expected. Or using `b.refresh_from_db()`. However I think
> it makes sense to fix this within the ORM because it's a subtle bug that
> can cause issues.

> This is a contrived example but the real use-case was cloning a model

> instance. And instead of erroring it just kept returning the cached value

New description:

Repro:

{{{
from django.db import models

class AppUser(models.Model):
pass

class Profile(models.Model):
user = models.OneToOneField(
to=AppUser,
on_delete=models.PROTECT,
)

a = AppUser.objects.create()
b = AppUser.objects.create()
Profile.objects.create(user=a)

b.profile # Errors, since it has not been created yet

a.profile.user_id = b.id
a.profile.save()

# Works!
Profile.objects.get(user=b)
# <Profile: Profile object (1)>

b.profile # Errors, since it uses the cached value
}}}

I can fix it by using `a.profile.user = b` instead of `a.profile.user_id =
b.id` which does the cache invalidation as expected. Or using
`b.refresh_from_db()` after the `save()` call. However I think it makes

sense to fix this within the ORM because it's a subtle bug that can cause
issues.

This is a contrived example but the real use-case was cloning a model

instance. And instead of erroring it just kept returning the cached value

--

--
Ticket URL: <https://code.djangoproject.com/ticket/31891#comment:3>

[Django]

#31891: Inconsistent related field cache invalidation
-------------------------------------+-------------------------------------

Reporter: Josh | Owner: nobody
Type: Bug | Status: new

Component: Uncategorized | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage:
orm,related,onetoone | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Josh):

* type: Uncategorized => Bug

--
Ticket URL: <https://code.djangoproject.com/ticket/31891#comment:4>

[Django]

#31891: Inconsistent related field cache invalidation
-------------------------------------+-------------------------------------

Reporter: Josh | Owner: nobody
Type: Bug | Status: new

Component: Database layer | Version: 3.1
(models, ORM) |

Severity: Normal | Resolution:
Keywords: | Triage Stage:
orm,related,onetoone | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Josh):

* component: Uncategorized => Database layer (models, ORM)

--
Ticket URL: <https://code.djangoproject.com/ticket/31891#comment:5>

[Django]

#31891: Remove cached value of reverse side of 020 relation when updating attname.

-------------------------------------+-------------------------------------
Reporter: Josh | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted
orm,related,onetoone |
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by felixxm):

* stage: Unreviewed => Accepted

Comment:

Thanks for this ticket, I can see inconsistency but I'm not sure if it's
feasible. Tentatively accepting for investigation.

The main difference is that `profile.user = b` uses one-to-one descriptors
and has access to both side instances, on the other hand `profile.user_id
= b.id` uses a deferred attribute.

> This is a contrived example but the real use-case was cloning a model
instance.

This is probably fixed by #31863.

--
Ticket URL: <https://code.djangoproject.com/ticket/31891#comment:6>

[Django]

#31891: Remove cached value of reverse side of 020 relation when updating attname.
-------------------------------------+-------------------------------------
Reporter: Josh | Owner:

| acampbell60
Type: Bug | Status: assigned

Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
orm,related,onetoone |

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by acampbell60):

* owner: nobody => acampbell60
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/31891#comment:7>