#34792: Creating and saving a model using a custom primary key field can yield a
bad "id" value on the instance
-------------------------------------+-------------------------------------
               Reporter:  mike w     |          Owner:  nobody
                   Type:             |         Status:  new
  Uncategorized                      |
              Component:  Database   |        Version:  4.2
  layer (models, ORM)                |       Keywords:  field, custom
               Severity:  Normal     |  fields
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 **Summary**: In some situations, creating and saving a model which uses a
 custom primary key can cause a wrong ".id" value to be set on the instance
 in memory.

 **Detail**: I am using a custom primary key Field type which inherits from
 AutoField, [https://github.com/mik3y/django-spicy-id django-spicy-id].
 Essentially what this Field does is translate the underlying integer data
 in the database, to and from a string (with a static prefix) at the Django
 level.

 It does this by implementing [https://github.com/mik3y/django-spicy-
 
id/blob/ce8cf54376f4cf0f82de9f385afaa70165eff2e3/django_spicy_id/fields.py#L147-L150
 from_db_value() and friends] in the Field implementation.

 This might not be to everyone's taste, but so far I think it is completely
 within the acceptable functionality & contract of custom Fields.

 I noticed that in some environments, particularly in Django pre-4.0,
 `save()` on a new instance would cause the instance's `.id` field to be
 the underlying number - and ''not'' the expected string value, which the
 custom Field implements. Example:

 {{{
 class MyModel(models.Model):
   id = SpicyBigAutoField("ex", primary_key=True, randomize=True)
 }}}

 {{{
 >>> o = MyModel()
 >>> o.save()
 >> o.id
 3735928559  # yikes! we should never get a number here.
 >>> o.refresh_from_db()
 >>> o.id
 'ex_deadbeef`
 }}}

 I believe the culprit is
 
[https://github.com/django/django/blob/fbd16438f46bc2128926958ad24331da5d1b406f/django/db/models/sql/compiler.py#L1825-L1847
 this block] in django.db.models.sql.compiler, specifically the last
 condition where self.connection.ops.last_insert_id is called and returned
 without modification.

 In turn, the parent caller
 
[https://github.com/django/django/blob/fbd16438f46bc2128926958ad24331da5d1b406f/django/db/models/base.py#L1057-L1063
 django.db.models.base.Model::_save_table()] will blindly setattr this
 value on the instance.

 It seems like a possible fix could be to flow the "last_insert_id" case
 through the Field's from_db_value(); in most cases this is surely a no-op,
 but.. not in this case.

 (I'd happy to prepare a patch and/or test if offered a little guidance.)
 Thanks!

-- 
Ticket URL: <https://code.djangoproject.com/ticket/34792>
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 on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a1f17984f-747bf5f3-27a1-402d-addf-820ff4154a22-000000%40eu-central-1.amazonses.com.

Reply via email to