id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
32420	build_instance in (de)serializers uses model's primary key field name instead of attname to detect if data contains a primary key	trybik	trybik	"The bug was introduced in #28385 fix, in [https://github.com/django/django/commit/dcd1025f4c03faa4a9915a1d47d07008280dd3cf commit dcd1025]. Currently in master, the {{{django.core.serializers.base.build_instance}}}  function uses {{{pk = data.get(Model._meta.pk.name)}}} instead of {{{pk = data.get(Model._meta.pk.attname)}}}:
{{{
def build_instance(Model, data, db):
    """"""
    Build a model instance.
    If the model instance doesn't have a primary key and the model supports
    natural keys, try to retrieve it from the database.
    """"""
    default_manager = Model._meta.default_manager
    pk = data.get(Model._meta.pk.name)
    if (pk is None and hasattr(default_manager, 'get_by_natural_key') and
            hasattr(Model, 'natural_key')):
        natural_key = Model(**data).natural_key()
        try:
            data[Model._meta.pk.attname] = Model._meta.pk.to_python(
                default_manager.db_manager(db).get_by_natural_key(*natural_key).pk
            )
        except Model.DoesNotExist:
            pass
    return Model(**data)
}}}

([https://github.com/django/django/blob/738e9e615dc81b561c9fb01439119f4475c2e25b/django/core/serializers/base.py#L260 Source on GitHub])

Why it should use {{{attname}}}? Because it may be different than {{{name}}} and the following corresponding assignments use {{{data[Model._meta.pk.attname] = ...}}}:
1. proceeding {{{django.core.serializers.python.Deserializer}}} which calls {{{build_instance}}}:
    {{{
    def Deserializer(...):
    ...
        for d in object_list:
            ...
            data = {}
            if 'pk' in d:
                try:
                    data[Model._meta.pk.attname] = Model._meta.pk.to_python(d.get('pk'))
                except Exception as e:
                ...
        obj = base.build_instance(Model, data, using)
        yield ...
    }}}
([https://github.com/django/django/blob/738e9e615dc81b561c9fb01439119f4475c2e25b/django/core/serializers/python.py#L100 Source on GitHub])

2. following 5 lines further assignment {{{ data[Model._meta.pk.attname] = ...}}} uses {{{attname}}}.


The marginal error case is when primary key is also a foreign key (then {{{attname}}} has {{{""_id""}}} suffix). Moreover, to actually make it an error you have to create a bit pathological situation where you have to have natural key but it does not work, e.g.:
{{{
class Author(models.Model):
    name = models.CharField(max_length=20)


class GhostWriterManager(models.Manager):

    def get_by_natural_key(self, *args, **kwargs):
        raise NotImplementedError(""Don't get by natural key"")


class GhostWriter(models.Model):
    author_ptr = models.ForeignKey(Author, on_delete=models.CASCADE, primary_key=True)

    objects = GhostWriterManager()

    def natural_key(self):
        raise NotImplementedError(""There is no natural key"")
}}}
This rare situation actually can come up in an arguably normal situation, when using ~~django-polymorphic~~ non-abstract subclasses - when loading the subclass part of JSON list, and when the subclass uses a natural key that refers to fields from base class. The natural key will work perfectly fine, just not when loading the subclass part of JSON."	Bug	assigned	Core (Serialization)	dev	Normal				Accepted	1	0	0	0	0	0