Opened 15 years ago
Last modified 21 months ago
#11265 new New feature
ForeignKey/OneToOneField should support user-defined id attribute name
Reported by: | dstora | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | |
Severity: | Normal | Keywords: | foreign key ForeignKey OneToOneField id |
Cc: | cmawebsite@…, elonzh | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, when defining a ForeignKey/OneToOneField field XXX, Django automatically defines a XXX_id attribute to store the id value.
However, it is sometimes desirable to decide of the name as XXX_id may not be the most appropriate.
Let's take the following example:
- I initially have a table "Payment" with a "ccy_code" field designating a 3-letter currency code.
My model definition looks like:
from django.db import model class Payment(models.Model): # ... ccy_code = models.CharField(max_length=3)
In my python code I refer to the currency code as follows:
p = Payment() #... print p.ccy_code
- Later, I decide to actually create a "Currency" table to hold some information about currencies.
And I also decide to define a foreign key constraint from "Payment" to "Currency".
My model now looks like:
from django.db import model class Currency(models.Model): ccy_code = models.CharField(max_length=3, primary_key=True) #... class Payment(models.Model): # ... ccy = models.ForeignKey(Currency, to_field="ccy_code", db_column="ccy_code")
The problem here is that my existing Python code is broken, because "ccy_code" is not defined anymore for "Payment".
Django has instead define a "ccy_id" attribute.
Based on the principle that defining things like foreign keys should only add functionality (and not remove any) it seems quite important to me that one can choose the id attribute name.
This can be achieved by adding an optional keyword argument to ForeignKey/OneToOneField constructors (here I decided to call it "id_attr_name").
The implementation of this feature is pretty small and fully backward-compatible.
Here is the SVN diff against trunk:
Index: related.py =================================================================== --- related.py (revision 10924) +++ related.py (working copy) @@ -660,6 +660,7 @@ class ForeignKey(RelatedField, Field): empty_strings_allowed = False def __init__(self, to, to_field=None, rel_class=ManyToOneRel, **kwargs): + self.__id_attr_name = kwargs.pop('id_attr_name', None) try: to_name = to._meta.object_name.lower() except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT @@ -679,7 +680,10 @@ self.db_index = True def get_attname(self): - return '%s_id' % self.name + if self.__id_attr_name: + return self.__id_attr_name + else: + return '%s_id' % self.name def get_validator_unique_lookup_type(self): return '%s__%s__exact' % (self.name, self.rel.get_related_field().name)
In my example, I could then define my model like:
from django.db import model class Currency(models.Model): ccy_code = models.CharField(max_length=3, primary_key=True) #... class Payment(models.Model): # ... ccy = models.ForeignKey(Currency, to_field="ccy_code", db_column="ccy_code", id_attr_name="ccy_code")
Attachments (1)
Change History (11)
by , 15 years ago
Attachment: | new_id_attribute_name_kwarg.patch added |
---|
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 15 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Triage Stage: | Accepted → Design decision needed |
Version: | 1.0 |
Needs tests.
Another thing to think about is does this need extra checks to ensure the id_attr_name
isn't stomping on any other attribute names?
comment:3 by , 15 years ago
Triage Stage: | Design decision needed → Accepted |
---|
Sorry, accepted is fine.
comment:4 by , 15 years ago
Not sure if this related. This is my models definition:-
from portal.models import User as DrupalUser class Profile(models.Model): profile_id = models.OneToOneField(DrupalUser, parent_link=False, primary_key=True) gid = models.ForeignKey(app_group.Group, db_column="gid") nama_penuh = models.CharField(max_length=128) class Meta: db_table = '"app_profile"."profile"' >>> from portal.models.app_profile import Profile >>> p = Profile.objects.get(pk=1) ProgrammingError: column profile.profile_id_id does not exist LINE 1: SELECT "app_profile"."profile"."profile_id_id", "app_profile...
profile_id
is the actual column name in table app_profile.profile
. My initial expectation is to use db_column
parameter just like the second definition for gid
field.
I try the patch (against 1.1 release which differ a bit) but got the following error:-
>>> from portal.models import User as DrupalUser >>> u = DrupalUser.objects.get(pk=1) >>> u.name u'admin' >>> u.profile Traceback (most recent call last): File "<console>", line 1, in <module> ...... File "/home/kamal/portal/trunk/django/env/lib/python2.5/site-packages/django/db/models/fields/related.py", line 273, in __set__ self.field.name, self.field.rel.to._meta.object_name)) ValueError: Cannot assign "1": "Profile.profile_id" must be a "User" instance.
Before I applied the above patch, there's no profile
attribute on the User
object.
comment:5 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:8 by , 10 years ago
Cc: | added |
---|
We're moving in the direction of having two distinct fields here. One that's simply an "id" field, and one that's the ForeignKey. This would control the name of the underlying field.
comment:9 by , 21 months ago
Cc: | added |
---|
comment:10 by , 21 months ago
This is funny, I have a model Biblio
with a issn
field, someday I created a new model IssnIndexedData
with issn
as primary key.
I want to join
IssnIndexedData
but it can not be done unless I add a logic Foreignkeyissn_indexed_data
withdb_column="issn"
toBiblio
. (See https://code.djangoproject.com/ticket/29262)
Fine, I just make a fake migration to make Django happy.
But wait! The attname
is hard coded and I can't use Biblio.issn
anymore...
Patch