[Django] #29896: Regression when saving a ForeignKey with to_field attr. Cached model is lost.
Django
lost.
-------------------------------------+-------------------------------------
Reporter: Maciej | Owner: nobody
Gol |
Type: Bug | Status: new
Component: Database | Version: 2.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 |
-------------------------------------+-------------------------------------
Hey guys, during recent patching run I've encountered regression between
Django 2.0.9 and 2.1.2.
Given models:
{{{
class Author(models.Model):
username = models.CharField(primary_key=True)
numeric_id = models.AutoField()
class Book(models.Model):
author = models.ForeignKey(Author, on_delete=models.Cascade)
author_numeric = models.ForeignKey(Author, on_delete=models.Cascade,
to_field="numeric_id")
}}}
The following test fails, although it shouldn't.
{{{
class SomeTest(TestCase):
def
test_foreign_key_with_to_field_should_properly_persist_through_save(self):
author = Author.objects.create(username="yada")
book = Book(author=author, author_numeric=author)
book.save()
with self.assertNumQueries(0):
book.author_numeric # No queries below Django 2.1. One query
for Django 2.1.
}}}
The problem comes from the `save()` method here:
{{{
# If the relationship's pk was changed, clear the cached
# relationship.
if obj and obj.pk != getattr(self, field.attname):
field.delete_cached_value(self)
}}}
We compare `obj.pk` with `book.author_numeric_id`, but `pk` is a string
(`username`) and `author_numeric_id` is an integer. It should look for
`to_field` value, instead:
{{{
# If the relationship's pk was changed, clear the cached
# relationship.
if obj and getattr(obj, field.target_field.attname) !=
getattr(self, field.attname):
field.delete_cached_value(self)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29896>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
lost.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.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:
Hey guys, during recent patching run I've encountered regression between
Django 2.0.9 and 2.1.2.
Given models:
{{{
class Author(models.Model):
username = models.CharField(primary_key=True)
numeric_id = models.AutoField()
class Book(models.Model):
author = models.ForeignKey(Author, on_delete=models.CASCADE)
author_numeric = models.ForeignKey(Author, on_delete=models.CASCADE,
to_field="numeric_id")
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29896#comment:1>
Django
lost.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the great analysis!
[https://github.com/django/django/pull/10573 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29896#comment:2>
Django
lost.
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/29896#comment:3>
Django
lost.
-------------------------------------+-------------------------------------
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"f77fc56c9671a2e2498e3920d79e247e6de18c16" f77fc56]:
{{{
#!CommitTicketReference repository=""
revision="f77fc56c9671a2e2498e3920d79e247e6de18c16"
Fixed #29896 -- Fixed incorrect Model.save() cache relation clearing for
foreign keys that use to_field.
Regression in ee49306176a2d2f1751cb890bd607d42c7c09196.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29896#comment:4>
Django
lost.
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"0f02d71995530f29a2d3246083854fe19697c3a5" 0f02d719]:
{{{
#!CommitTicketReference repository=""
revision="0f02d71995530f29a2d3246083854fe19697c3a5"
[2.1.x] Fixed #29896 -- Fixed incorrect Model.save() cache relation
clearing for foreign keys that use to_field.
Regression in ee49306176a2d2f1751cb890bd607d42c7c09196.
Backport of f77fc56c9671a2e2498e3920d79e247e6de18c16 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29896#comment:5>