Opened 5 years ago
Closed 5 years ago
#31499 closed Cleanup/optimization (fixed)
Store ModeState.fields into a dict.
Reported by: | Simon Charette | Owned by: | Simon Charette |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
ModeState
initially stored its fields
into a List[Tuple[str, models.Field]]
because it wanted to preserve ordering.
However the auto-detector doesn't consider field re-ordering as a state change and Django doesn't support table column reordering in the first place. The only reason I'm aware of for keeping field ordering is to generate model forms out of them which is unlikely to happen during migrations and if it was the case the only the order in which field are ordered and validated would change if Meta.fields = '__all__
is used which is discouraged.
Given storing fields this way results in awkward and inefficient lookup by name for no apparent benefits and that dict
now preserves insertion ordering I suggest we switch ModelState.fields
to Dict[str, models.Field]
. I suggest we do the same for ModelState.indexes
and .constraints
since they suggest from the same awkwardness which was likely cargo culted from ModelState.fields
design decision.
Change History (5)
comment:2 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 5 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
PR