Opened 6 months ago

Last modified 2 months ago

#27944 assigned Cleanup/optimization

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: master
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 (5)

comment:1 Changed 6 months ago by Tim Graham

Component: UncategorizedDatabase layer (models, ORM)
Triage Stage: UnreviewedAccepted

comment:2 Changed 6 months ago by Josh Schneier

Has patch: set
Owner: changed from nobody to Josh Schneier
Status: newassigned

comment:3 Changed 6 months ago by Tim Graham

Patch needs improvement: set

comment:4 Changed 6 months ago by Josh Schneier

Patch needs improvement: unset

comment:5 Changed 2 months ago by Tim Graham

Triage Stage: AcceptedSomeday/Maybe

The discussion on the PR suggests that a DevelopersMailingList discussion is needed to see if there's consensus to proceed with this.

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