Opened 6 years ago

Closed 6 years ago

#29900 closed Cleanup/optimization (wontfix)

ForeignKey foo with to_field=bar option: Accessor should be called foo_bar not foo_id

Reported by: Paul Zeinlinger Owned by: nobody
Component: Database layer (models, ORM) Version: 2.1
Severity: Normal Keywords: to_field
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have the following PipeItem model:

class PipeItem(models.Model):
    entity_uuid = models.UUIDField(db_index=True, default=uuid4, editable=False)
    user = models.ForeignKey(
        User,
        db_column="user_entity_uuid",
        to_field="entity_uuid",
        on_delete=models.CASCADE,
    )

Unfortunately, PipeItem.create(user_entity_uuid="XXXX") does not accept the keyword argument user_entity_uuid, but just user_id (which is an uuid in this case).
I propose to change the direct accessor name to the name of the corresponding to_field.

Change History (3)

comment:1 by Simon Charette, 6 years ago

I agree that it could be more intuitive to use f'{field.name}_{field.target_field.name}' as attribute name. However changing it at this point would break backward compatibility for a large number of projects and require a deprecation period.

Also in the cases where it's the referenced primary key that declares a different name the reference attribute name would be less obvious because of the implicit to_field. The ambiguity is amplified when the referenced model is declared in another app.

# app1/model.py
class User(models.Model):
    username = models.CharField(primary_key=True)

# app2/model.py
class Message(models.Model):
    user = models.ForeignKey(User)  # Implicit to_field='username'

Message.objects.create(user_id='foo')  # Would be a TypeError
Message.objects.create(user_username='foo')

I find it kind of practical that foreign key attribute names are simply f'{field.name}_id' independently of the referenced field name. In a sense it's kind of how pk is an alias for whatever the primary key name is. That kind of defeats the purity argument IMO.

Version 2, edited 6 years ago by Simon Charette (previous) (next) (diff)

comment:2 by Paul Zeinlinger, 6 years ago

Well, you are absolutely right about the breaking change. It would certainly require a deprecation period. But since f'{field.name}_{field.target_field.name}' is not in use right now, we could alias it and keep backward compatibility in this way.
IMHO your example demonstrates exactly, why we should not keep doing it this way. In your example, we don't have any information about the contents or the type of user_id. It could as well be a UUID, a text field with the user's bio or an integer.

In case of a pk, we know that there's only one in a table. But ForeignKeys with to_fields can reference any field, not just the primary key (as long as it is unique). So I guess, it's not practical to compare those two cases.

Let's consider another example:
If we have an API serving messages as json to the user, but we don't want to disclose the primary key of the user (eg. for security reasons), then we could serve a json with a {.... user_uuid: XXX} entry. However, the api user wouldn't be able to set the corresponding database user directly - he would need to submit the users UUID, we would need to look it up in the db, get the username and set it, respectively (that's 3 lookups compared to just one).

I can't see a point, why this would be more intuitive than just having a corresponding field.
But I might be wrong :-) It's just an idea.

comment:3 by Tim Graham, 6 years ago

Resolution: wontfix
Status: newclosed

I guess this would have to be discussed on the DevelopersMailingList to see if there's consensus to go through the hassle of changing the accessor name. My guess is that it would cause more trouble than it's worth at this point.

A related ticket is #11265, "ForeignKey/OneToOneField should support user-defined id attribute name".

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