Opened 7 years ago

Closed 21 months 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 Tim Graham, 7 years ago

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

comment:2 by Josh Schneier, 7 years ago

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

comment:3 by Tim Graham, 7 years ago

Patch needs improvement: set

comment:4 by Josh Schneier, 7 years ago

Patch needs improvement: unset

comment:5 by Tim Graham, 7 years ago

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.

comment:6 by Simon Charette, 21 months ago

Resolution: wontfix
Status: assignedclosed

I think this can be closed as wontfix given the discussion on the mailing list didn't reach consensus around the backward compatibility risks.

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