Opened 21 months ago
Last modified 2 months ago
#34569 assigned Cleanup/optimization
Unify all model fields to call get_prep_value from get_db_prep_value
Reported by: | Julie Rymer | Owned by: | stimver |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Pull Requests: | How to create a pull request | ||
Description ¶
Following on #34539, should all fields that do not currently call get_prep_value
from get_db_prep_value
be updated to call it so that we have no more inconsistency with the fields customization doc?
Here is the list of all concerned fieldst:
- AreaField (contrib.gis)
- DistanceField (contrib.gis)
- ArrayField (contrib.postgres)
- DurationField
- UUIDField (this one even has a definition for
get_prep_value
, but it is never called) - ForeignKey (like previous one, possess definition for
get_prep_value
, never called)
Please give me your thoughts or if I missed anything.
I for one am in favour of harmonising all field to follow the field API
(PS: wasn't sure about what ticket type this should be classified as)
According to the ticket's flags, the next step(s) to move this issue forward are:
- To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is:
[https://github.com/django/django/pull/#### PR]
.
Change History (4)
comment:1 by , 21 months ago
Type: | Uncategorized → Cleanup/optimization |
---|
comment:3 by , 21 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 20 months ago
Triage Stage: | Unreviewed → Accepted |
---|
Tentatively accepted, but for concrete clases not as a rule. IMO, we definitely shouldn't do this for ForeignKey
and its subclasses.
I knew that it'd open Pandora's box 😞 IMO we gain nothing from adding:
to all these fields, but the community can decide otherwise. We have to be very carefully with
ForeignKey
which has many subclasses.That's not true, see a coverage report.
That's not true, see a coverage report.