Opened 7 years ago
Closed 6 years ago
#29085 closed Cleanup/optimization (duplicate)
Possible data loss on .save() with unsaved related model.
Reported by: | Jonas Haag | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
- Create unsaved model C
- Assign unsaved related model P
- Save related model P and C
- Relationship is lost
This is similar to #25160 I guess?
I reproduced this with all versions of Django 1.6 up to master.
from django.db import models class Parent(models.Model): pass class Child(models.Model): parent = models.ForeignKey(Parent, null=True, blank=True, on_delete=models.CASCADE)
from .models import Parent, Child from django.test import TestCase class MyTest(TestCase): def test_save(self): child = Child() child.parent = Parent() child.parent.save() # This makes the problem go away: # child.parent = child.parent child.save() child.refresh_from_db() self.assertIsNotNone(child.parent)
Attachments (1)
Change History (9)
comment:1 by , 7 years ago
Type: | Uncategorized → Cleanup/optimization |
---|
comment:2 by , 7 years ago
Triage Stage: | Unreviewed → Accepted |
---|
by , 7 years ago
Attachment: | patch.diff added |
---|
comment:3 by , 7 years ago
comment:4 by , 7 years ago
Without knowing what an implementation might look like, I'd expect the save to work since the parent is saved before the child.
comment:6 by , 7 years ago
I believe I hit the same issue today.
I have a set of related objects that I want to construct all of the forms for, validate all of them, and then save.
However, the reference to the primary object (which is still "connected" to the child instances) is removed when saving.
I _think_ it's got to do with Field.pre_save(self, model_instance, add)
being called before saving: which uses field.attname
, which is necessarily empty initially (because we don't have a primary key on the reference object, yet).
I'm going to play around, but I think it could be that a ForeignKey can have slightly different behaviour, where it falls back to the related (in-memory) object, if one exists, and field.attname
is None.
comment:7 by , 6 years ago
I've been looking into this a bit and I don't see how we can do the right thing here. The problem I see is that the Child
model has two sources of truth about the Parent
instance and we don't know which we can trust. These sources are Child.parent_id
and Child.parent.id
. When we save the Parent
instance, Child.parent.id
changes, but Child.parent_id
does not and I don't see a way to push the new pk
to Child.parent_id
behind the scenes. One can reassign Child.parent
explicitly (as noted above), which ensures Child.parent_id
is correct.
I looked into adjusting ForeignKey.pre_save
to read from Child.parent
and Model.save
to avoid clearing Child._state.fields_cache
, but this either didn't work or broke many other Django tests.
comment:8 by , 6 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Summary: | Possible data loss on .save() with unsaved related model → Possible data loss on .save() with unsaved related model. |
Duplicate of #28147.
Is this the correct approach? Or should we rather consider the behaviour a bug and not use the "prohibited error" in the first place?