Saving parent object after setting on child leads to data loss for parents with non-numeric primary key.

[Charlie DeTar]

Given a model with a foreign key relation to another model that has a non-auto CharField as its primary key:

class Product(models.Model):
    sku = models.CharField(primary_key=True, max_length=50)

class Order(models.Model):
    product = models.ForeignKey(Product, on_delete=models.CASCADE)

If the relation is initialized on the parent with an empty instance that does not yet specify its primary key, and the primary key is subsequently defined, the parent does not "see" the primary key's change:

with transaction.atomic():
    order = Order()
    order.product = Product()
    order.product.sku = "foo"
    order.product.save()
    order.save()

    assert Order.objects.filter(product_id="").exists()   # Succeeds, but shouldn't
    assert Order.objects.filter(product=order.product).exists()  # Fails

Instead of product_id being populated with product.sku, it is set to emptystring. The foreign key constraint which would enforce the existence of a product with sku="" is deferred until the transaction commits. The transaction does correctly fail on commit with a ForeignKeyViolation due to the non-existence of a product with emptystring as its primary key.

On the other hand, if the related unsaved instance is initialized with its primary key before assignment to the parent, it is persisted correctly:

with transaction.atomic():
    order = Order()
    order.product = Product(sku="foo")
    order.product.save()
    order.save()

    assert Order.objects.filter(product=order.product).exists()  # succeeds

Committing the transaction also succeeds.

This may have something to do with how the Order.product_id field is handled at assignment, together with something about handling fetching of auto vs non-auto primary keys from the related instance.

[Mariusz Felisiak]

Thanks for this report. product_id is an empty string in ​_prepare_related_fields_for_save() that's why pk from a related object is not used. We could use empty_values:

diff --git a/django/db/models/base.py b/django/db/models/base.py
index 822aad080d..8e7a8e3ae7 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -933,7 +933,7 @@ class Model(metaclass=ModelBase):
                         "%s() prohibited to prevent data loss due to unsaved "
                         "related object '%s'." % (operation_name, field.name)
                     )
-                elif getattr(self, field.attname) is None:
+                elif getattr(self, field.attname) in field.empty_values:
                     # Use pk from related object if it has been saved after
                     # an assignment.
                     setattr(self, field.attname, obj.pk)

but I'm not sure.

Related with #28147.

[Hasan Ramezani]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7cba92ec:

Fixed #32332 -- Fixed loss of parent with non-numeric pk when saving child after parent.

Follow up to 519016e5f25d7c0a040015724f9920581551cab0.

[Mariusz Felisiak <felisiak.mariusz@…>]

In b36beec:

[3.2.x] Fixed #32332 -- Fixed loss of parent with non-numeric pk when saving child after parent.

Follow up to 519016e5f25d7c0a040015724f9920581551cab0.

Backport of 7cba92ec55a5d05450261f375830619870ca84fa from master