Opened 8 years ago
Closed 2 years ago
#27944 closed Cleanup/optimization (wontfix)
Have meta.get_field('pk') return the primary key field directly
Reported by: | Josh Schneier | Owned by: | Josh Schneier |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Someday/Maybe | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Follow up ticket discussion on this topic: https://github.com/django/django/pull/8148
While fixing #27897, which is caused by calling .get_field('pk')
, I ran across the fact that we are now checking == 'pk'
before calling .get_field
in several places. My proposal is to simply handle this special case inside of get_field
directly.
I have a patch but there is a single test regression on this system check. My idea was to change the message and obj
to self.model
but I'm not sure what the backwards compatibility guarantees are for the message.
Regardless I think we should do this because it will prevent a whole class of bugs in the future and 'pk'
is already special cased everywhere.
Change History (6)
comment:1 by , 8 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 8 years ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 8 years ago
Patch needs improvement: | set |
---|
comment:4 by , 8 years ago
Patch needs improvement: | unset |
---|
comment:5 by , 7 years ago
Triage Stage: | Accepted → Someday/Maybe |
---|
The discussion on the PR suggests that a DevelopersMailingList discussion is needed to see if there's consensus to proceed with this.
comment:6 by , 2 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
I think this can be closed as wontfix given the discussion on the mailing list didn't reach consensus around the backward compatibility risks.
Patch here: https://github.com/django/django/pull/8191