[Django] #29085: Possible data loss on .save() with unsaved related model

Django

unread,

Jan 29, 2018, 2:28:31 PM1/29/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------
Reporter: Jonas | Owner: nobody
Haag |
Type: | Status: new
Uncategorized |
Component: Database | Version: master
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 |
-------------------------------------+-------------------------------------
- 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)
}}}

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

Django

unread,

Jan 29, 2018, 2:28:43 PM1/29/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

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

Changes (by Jonas Haag):

* type: Uncategorized => Cleanup/optimization

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

Django

unread,

Jan 29, 2018, 2:50:45 PM1/29/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

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

Changes (by Tim Graham):

* stage: Unreviewed => Accepted

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

Django

unread,

Jan 30, 2018, 4:00:10 PM1/30/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

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

Changes (by Jonas Haag):

* Attachment "patch.diff" added.

Django

unread,

Jan 30, 2018, 4:01:38 PM1/30/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

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

Comment (by Jonas Haag):

Is this the correct approach? Or should we rather consider the behaviour a
bug and not use the "prohibited error" in the first place?

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

Django

unread,

Jan 30, 2018, 6:02:05 PM1/30/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

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

Comment (by Tim Graham):

Without knowing what an implementation might look like, I'd expect the
save to work since the parent is saved before the child.

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

Django

unread,

Jun 15, 2018, 2:14:22 PM6/15/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

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

Comment (by Tim Graham):

#29497 is a duplicate.

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

Django

unread,

Jun 19, 2018, 12:50:35 AM6/19/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

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

Comment (by Matthew Schinckel):

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.

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

Django

unread,

Nov 13, 2018, 11:41:56 AM11/13/18

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model
-------------------------------------+-------------------------------------

Has patch: 0 | Needs documentation: 0

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

Comment (by Ian Foote):

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.

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

Django

unread,

May 24, 2019, 5:40:55 AM5/24/19

to django-...@googlegroups.com

#29085: Possible data loss on .save() with unsaved related model.

-------------------------------------+-------------------------------------
Reporter: Jonas Haag | Owner: nobody

Type: | Status: closed

Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |

Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

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