Opened 10 years ago

Closed 8 years ago

Last modified 6 months ago

#22936 closed Cleanup/optimization (fixed)

Get rid of field.get_db_prep_lookup()

Reported by: Anssi Kääriäinen Owned by: nobody
Component: Database layer (models, ORM) 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 (last modified by Tim Graham)

The Field.get_db_prep_lookup() method is used to do preparation of a plain Python value for given lookup. Looking at the coding of get_db_prep_lookup() the get_db_prep_lookup() is doing work that belongs into the lookup itself.

The same goes also for get_prep_lookup(). It seems that get_prep_lookup() is either doing something that should actually be done in get_prep_value(), or something that belongs into lookups.

If a custom field needs to do different kind of preparation for some lookups (something that can't be done in get_[db_]prep_value(), it can always provide a lookup subclass that does the right thing for that field type. An example is IntegerField and 'lt' and 'gte' lookups. Providing custom subclasses might be laborious to do if there are a lot of different lookup types that need custom preparation. For that reason a hook for custom fields could still be useful.

Even if we want to leave get_db_prep_lookup and get_prep_lookup hooks for custom fields, the base coding belongs into the lookups themselves. Currently the fields are doing work that clearly belongs to the lookup itself.

Change History (11)

comment:1 by Tim Graham, 10 years ago

Description: modified (diff)
Triage Stage: UnreviewedAccepted

comment:2 by Tim Graham, 8 years ago

Description: modified (diff)

As suggested in the ticket description, I did the small step of moving IntegerField.get_prep_lookup() to lookup subclasses: PR.

comment:3 by Tim Graham <timograham@…>, 8 years ago

In 1c30a647:

Refs #22936 -- Moved IntegerField.get_prep_lookup() logic to lookups.

comment:4 by Claude Paroz, 8 years ago

This PR removes the get_prep_lookup method.

comment:5 by Tim Graham <timograham@…>, 8 years ago

In 1ba0b22:

Refs #22936 -- Removed unused code in Field.get_db_prep_lookup().

comment:6 by Tim Graham <timograham@…>, 8 years ago

In eab5df1:

Refs #22936 -- Moved more of Field.get_db_prep_lookup() to lookups.

comment:7 by Tim Graham, 8 years ago

Has patch: set

I submitted a PR to Claude's PR in comment:4 which completes the ticket.

comment:8 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Claude Paroz <claude@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 388bb5b:

Fixed #22936 -- Obsoleted Field.get_prep_lookup()/get_db_prep_lookup()

Thanks Tim Graham for completing the initial patch.

comment:10 by GitHub <noreply@…>, 6 months ago

In 91cb2d0:

Refs #22936 -- Doc'd Lookup.prepare_rhs.

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 6 months ago

In 6e7c8cc:

[5.0.x] Refs #22936 -- Doc'd Lookup.prepare_rhs.

Backport of 91cb2d0b487acc56d886612a7251b9ba555d71b4 from main

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