Opened 9 years ago

Closed 9 years ago

#25279 closed New feature (fixed)

Make prefetch_related_objects a public API

Reported by: Adam Johnson Owned by: nobody
Component: Database layer (models, ORM) Version: 1.8
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)

prefetch_related is great, but it would be nice to be able to reuse its capabilities with lists of model instances already retrieved by other code (or from cache, or newly constructed, etc.). It seems this is possible by using django.db.models.query.prefetch_related_objects which is the function implementing all the prefetch capability - I've just seen it used fine, however it is not part of Django's public API.

As far as I can tell, just doing an import in django.db.models.__init__ and adding documentation should be enough to make it public, since prefetch_related_objects contains all the functionality that prefetch_related exposes.

django-developers discussion

Change History (10)

comment:1 by Tim Graham, 9 years ago

Seems related to #24975 which proposes to add the ability to call prefetch_related() on a model instance. I wonder if we could find a common solution to address both of these tickets.

If we make prefetch_related_objects() public, I think some tests would also be needed to ensure that it isn't refactored so that it's signature changes.

comment:2 by Adam Johnson, 9 years ago

Has patch: set

Oh #24975 is nice. It doesn't seem that hard to do - on Model would it not just require...

def prefetch_related(self, related_lookups):
    prefetch_related_objects([self], related_lookups)

Basic PR at https://github.com/django/django/pull/5138, with a documentation sketch. Agree that more tests would be required.

comment:3 by Tim Graham, 9 years ago

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:4 by Adam Johnson, 9 years ago

I've added some tests for the basics (one-to-one/many-to-many traversal plus Prefetch objects) but don't want to go too far basically duplicating the tests that already exist for prefetch_related. The aim is just to check it accepts the same arguments and thought is put in to changing it in the future.

I've also changed the function signature to prefetch_related_objects(list_of_objects, *lookups) for the simpler invocation prefetch_related_objects(objs, 'lookup1', 'lookup2__and3'), as opposed to prefetch_related_objects(objs, ['lookup1', 'lookup2__and3']). This is to defend against the possible bug from prefetch_related_objects(objs, 'lookup1') which would iterate the characters in lookup1 leading to errors about attribute l etc. not existing.

I'm guessing that although some existing projects may already be using prefetch_related_objects, it's not Django's policy to worry about changing the signature of a (previously) private function? Or should I add type-checking like if len(args) == 1 and isinstance(args[0], (tuple, list)) to allow the old style?

comment:5 by Tim Graham, 9 years ago

Needs tests: unset

comment:6 by Tim Graham, 9 years ago

Description: modified (diff)

comment:7 by Tim Graham, 9 years ago

Keywords: 1.9 added

Loic had some doubts about making prefetch_related_objects() a public API. So, I'm awaiting his feedback here or on the mailing list before proceeding with this ticket.

comment:8 by Tim Graham, 9 years ago

Keywords: 1.9 removed

Loic's concerns are, "I have nothing against exposing that functionality itself, I just wanted to review if it works as-is as an elegant and reliable public API, right now it's a utility function it wasn't designed with those concerns in mind."

Since it's mostly a documentation patch at this point, it doesn't seem critical for 1.9.

comment:9 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

Pending Loic's review.

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

Resolution: fixed
Status: newclosed

In ef33bc2:

Fixed #25279 -- Made prefetch_related_objects() public.

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