Model attribute cannot be named pk and set primary_key=True

[Jon Gales]

I had an existing database that I wanted to manipulate with Django and ran in through the inspectdb management command. Opening up the admin to try and edit the table led to a consistent Python crash (OS X popped up the crash log which I attached). After trial and error I narrowed it down to the name of the model attribute: "pk". Changing the attribute name to something else and then setting db_column='pk' allowed everything to function normally.

When accessing the model through the shell Python doesn't crash, but no data is returned on queries and nothing can be inserted. Here's an example model that shows the problem, run syncdb to set it up and then try to interact with it.

from django.db import models

class Example(models.Model):
    pk = models.AutoField(primary_key=True, db_column='pk')

---

>> test = Example()

File "/Users/jonknee/src/envs/django_1.2/lib/python2.6/site-packages/django/db/models/base.py", line 403, in _set_pk_val
    return setattr(self, self._meta.pk.attname, value)
RuntimeError: maximum recursion depth exceeded while calling a Python object

Changing the attribute name makes everything work normally:

class Example(models.Model):
    pkey = models.AutoField(primary_key=True, db_column='pk')

This is an issue in both 1.1 and 1.2, I'm using Python 2.6.1.

[Jon Gales]

Python crash log

[Alex Gaynor]

Fixed up formatting. Also this is just going to be a docs issue IMO.

[Karen Tracey]

Replying to Alex:

Fixed up formatting. Also this is just going to be a docs issue IMO.

Well, inspectdb could be made smarter and attempt to avoid the problem. #12460 is already open for some improvements along those lines to prevent generation of bad field names. Also if you manually attempt to name a field pk that could get flagged as an error...

[Jon Gales]

Replying to kmtracey:

#12460 is already open for some improvements along those lines to prevent generation of bad field names. Also if you manually attempt to name a field pk that could get flagged as an error...

In a related issue, Django doesn't throw up any roadblocks to calling a non primary key field pk which leads to odd behavior (the attribute pk and the actual primary key can't be separately set). Flagging that as an error gets a +1 from me. It's not a good thing when valid Python takes the ship down, doubly so when it's code created by inspectdb and a table cleanly made by syncdb. It took me a long time to figure out because the crashlog had nothing that hinted at what it was.

[hjeffrey]

I think that this is at least a partial fix. The core should throw an exception when you name a model attribute "pk"

[Luke Plant]

This is the wrong place to put the check, since it will only trigger when the table is created by syncdb, which is not a necessary step for using a Django model. It should probably be in django/core/management/validation.py, with tests in modeltests/invalid_models/models.py

Also, AttributeError would certainly not be the right exception to raise - ​http://docs.python.org/library/exceptions.html#exceptions.AttributeError

By the way, you should not normally set 'Ready for checkin' yourself, but rather get someone else to review your patch and set that.

[Jacob]

Milestone 1.3 deleted

[Olivier Le Thanh Duong]

Looks like this bug was fixed in ee9fcb1672ddf5910ed8c45c37a00f32ebbe2bb1