Opened 12 years ago

Closed 10 years ago

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

Description: modified (diff)
Triage Stage: Unreviewed → Accepted

comment:2 by Tim Graham, 10 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@…>, 10 years ago

In 1c30a647:

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

comment:4 by Claude Paroz, 10 years ago

​This PR removes the get_prep_lookup method.

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

In 1ba0b22:

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

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

In eab5df1:

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

comment:7 by Tim Graham, 10 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, 10 years ago

Triage Stage: Accepted → Ready for checkin

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

Resolution: → fixed
Status: new → closed

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@…>, 3 years ago

In 91cb2d0:

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

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years 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