Opened 3 years ago

Last modified 3 years 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


Follow up ticket discussion on this topic:

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 3 years ago by Tim Graham

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

comment:2 Changed 3 years ago by Josh Schneier

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

comment:3 Changed 3 years ago by Tim Graham

Patch needs improvement: set

comment:4 Changed 3 years ago by Josh Schneier

Patch needs improvement: unset

comment:5 Changed 3 years 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