Opened 6 years ago

Last modified 5 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@… 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)

new_id_attribute_name_kwarg.patch (1004 bytes) - added by dstora 6 years ago.
Patch

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by dstora

Patch

comment:1 Changed 6 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by SmileyChris

  • Needs documentation set
  • Needs tests set
  • Triage Stage changed from Accepted to Design decision needed
  • Version 1.0 deleted

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 Changed 6 years ago by SmileyChris

  • Triage Stage changed from Design decision needed to Accepted

Sorry, accepted is fine.

comment:4 Changed 6 years ago by k4ml

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 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:6 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:7 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:8 Changed 5 months ago by collinanderson

  • Cc cmawebsite@… 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.

Note: See TracTickets for help on using tickets.
Back to Top