[Django] #29900: ForeignKey foo with to_field=bar option: Accessor should be called foo_bar not foo_id

5 views
Skip to first unread message

Django

unread,
Oct 28, 2018, 4:56:20 AM10/28/18
to django-...@googlegroups.com
#29900: ForeignKey foo with to_field=bar option: Accessor should be called foo_bar
not foo_id
-------------------------------------+-------------------------------------
Reporter: Paul | Owner: nobody
Zeinlinger |
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: 2.1
layer (models, ORM) |
Severity: Normal | Keywords: to_field
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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.

--
Ticket URL: <https://code.djangoproject.com/ticket/29900>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Oct 28, 2018, 10:12:16 AM10/28/18
to django-...@googlegroups.com
#29900: ForeignKey foo with to_field=bar option: Accessor should be called foo_bar
not foo_id
-------------------------------------+-------------------------------------
Reporter: Paul Zeinlinger | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: to_field | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

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 name that declares
a different name the attribute name would be less obvious because of the
implicit `to_field`. The problematic is even worse when the referenced
model is declared in another app.

{{{#!python
# 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.

--
Ticket URL: <https://code.djangoproject.com/ticket/29900#comment:1>

Django

unread,
Oct 28, 2018, 11:21:42 AM10/28/18
to django-...@googlegroups.com
#29900: ForeignKey foo with to_field=bar option: Accessor should be called foo_bar
not foo_id
-------------------------------------+-------------------------------------
Reporter: Paul Zeinlinger | Owner: nobody
Type: | Status: new
Cleanup/optimization |

Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: to_field | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Paul Zeinlinger):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/29900#comment:2>

Django

unread,
Nov 3, 2018, 11:33:45 AM11/3/18
to django-...@googlegroups.com
#29900: ForeignKey foo with to_field=bar option: Accessor should be called foo_bar
not foo_id
-------------------------------------+-------------------------------------
Reporter: Paul Zeinlinger | Owner: nobody
Type: | Status: closed

Cleanup/optimization |
Component: Database layer | Version: 2.1
(models, ORM) |
Severity: Normal | Resolution: wontfix

Keywords: to_field | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* status: new => closed
* resolution: => wontfix


Comment:

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".

--
Ticket URL: <https://code.djangoproject.com/ticket/29900#comment:3>

Reply all
Reply to author
Forward
0 new messages