[Django] #32332: Relation with non-auto CharField primary key is not correctly persisted if the primary key is defined after assignment
Django
the primary key is defined after assignment
-------------------------------------+-------------------------------------
Reporter: Charlie | Owner: nobody
DeTar |
Type: Bug | Status: new
Component: Database | Version: 3.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 |
-------------------------------------+-------------------------------------
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=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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32332>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
the primary key is defined after assignment
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 3.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:
> Given a model with a foreign key relation to another model that has a
> non-auto CharField as its primary key:
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32332#comment:1>
Django
with non-numeric primary key.
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: robinh00d, Jon Dufresne (added)
Comment:
Thanks for this report. `product_id` is an empty string in
[https://github.com/django/django/blob/2d6179c819010f6a9d00835d5893c4593c0b85a0/django/db/models/base.py#L936-L939
_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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32332#comment:2>
Django
with non-numeric primary key.
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: nobody
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32332#comment:3>
Django
with non-numeric primary key.
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner:
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => sohailtanveer01
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32332#comment:4>
Django
with non-numeric primary key.
-------------------------------------+-------------------------------------
| Ramezani
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: sohail tanveer => Hasan Ramezani
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/13964 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/32332#comment:5>
Django
with non-numeric primary key.
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Hasan
| Ramezani
Type: Bug | Status: assigned
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | 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/32332#comment:6>
Django
with non-numeric primary key.
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Hasan
| Ramezani
Component: Database layer | Version: 3.1
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"7cba92ec55a5d05450261f375830619870ca84fa" 7cba92ec]:
{{{
#!CommitTicketReference repository=""
revision="7cba92ec55a5d05450261f375830619870ca84fa"
Fixed #32332 -- Fixed loss of parent with non-numeric pk when saving child
after parent.
Follow up to 519016e5f25d7c0a040015724f9920581551cab0.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32332#comment:7>
Django
with non-numeric primary key.
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Hasan
| Ramezani
Type: Bug | Status: closed
Component: Database layer | Version: 3.1
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b36beec2086da34b7bb8fee0001edf47a41716fb" b36beec]:
{{{
#!CommitTicketReference repository=""
revision="b36beec2086da34b7bb8fee0001edf47a41716fb"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32332#comment:8>