Opened 10 months ago
Last modified 10 months ago
#36074 closed Bug
Updating a model instance with a composite primary key through .save() results in unnecessary re-assignment of primary key fields — at Initial Version
| Reported by: | Simon Charette | Owned by: | Simon Charette |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Release blocker | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Looking at the code that gets involved and the generated SQL It appears that every save that results in an UPDATE for a model with a composite primary key also includes all the primary key field in the updated fields.
For example, for a model of the form
class User(models.Model): tenant = models.ForeignKey(Tenant) id = models.UUIDField(default=uuid.uuid4) username = models.EmailField()
Then doing
user = User.objects.create(tenant=tenant, id=1, email="foo@bar.org") user.email = "something@example.com" user.save() # Will result in UPDATE user SET tenant_id = %s, id = %s, email = %s WHERE tenant_id = %s AND id = %s # ^^^^^^^^^^^^^^^^^^^^^^^ <- tenant_id and id should not be marked for UPDATE
Will result in the following SQL
INSERT INTO user(tenant_id, id, email) VALUES (%s, %s, %s) UPDATE user SET tenant_id = %s, id = %s, email = %s WHERE tenant_id = %s AND id = %s
But the UPDATE statement should not set tenant_id and id as they are part of the primary key lookup. The UPDATE should be
UPDATE user SET email = %s WHERE tenant_id = %s AND id = %s
which is due to a non-audited usage of .primary_key in `save_tables
-
django/db/models/base.py
diff --git a/django/db/models/base.py b/django/db/models/base.py index a7a26b405c..6d66080c20 100644
a b def _save_table( 1091 1091 for a single table. 1092 1092 """ 1093 1093 meta = cls._meta 1094 pk_fields = meta.pk_fields 1094 1095 non_pks_non_generated = [ 1095 1096 f 1096 1097 for f in meta.local_concrete_fields 1097 if not f.primary_keyand not f.generated1098 if f not in pk_fields and not f.generated 1098 1099 ] 1099 1100 1100 1101 if update_fields:
and makes me fear there might be others lying around that went unnoticed.