#36074: Updating a model instance with a composite primary key through .save()
results in unnecessary re-assignment of primary key fields
-------------------------------------+-------------------------------------
               Reporter:  Simon      |          Owner:  Simon Charette
  Charette                           |
                   Type:  Bug        |         Status:  assigned
              Component:  Database   |        Version:  dev
  layer (models, ORM)                |
               Severity:  Release    |       Keywords:
  blocker                            |
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 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

 {{{#!python
 class User(models.Model):
     tenant = models.ForeignKey(Tenant)
     id = models.UUIDField(default=uuid.uuid4)
     username = models.EmailField()
 }}}

 Then doing

 {{{#!python
 user = User.objects.create(tenant=tenant, id=1, email="[email protected]")
 user.email = "[email protected]"
 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

 {{{#!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


 {{{#!sql
 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


 {{{#!diff
 diff --git a/django/db/models/base.py b/django/db/models/base.py
 index a7a26b405c..6d66080c20 100644
 --- a/django/db/models/base.py
 +++ b/django/db/models/base.py
 @@ -1091,10 +1091,11 @@ def _save_table(
          for a single table.
          """
          meta = cls._meta
 +        pk_fields = meta.pk_fields
          non_pks_non_generated = [
              f
              for f in meta.local_concrete_fields
 -            if not f.primary_key and not f.generated
 +            if f not in pk_fields and not f.generated
          ]

          if update_fields:
 }}}

 and makes me fear there might be others lying around that went unnoticed.
-- 
Ticket URL: <https://code.djangoproject.com/ticket/36074>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/django-updates/010701944989f8b2-b8602df6-0a4c-429f-a728-68459daf5709-000000%40eu-central-1.amazonses.com.

Reply via email to