Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#28600 closed New feature (fixed)

Add prefetch related support to RawQuerySet

Reported by: Adnan Umer Owned by: Adnan Umer
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Дилян Палаузов, Carlton Gibson 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 Adnan Umer)

When using custom raw SQL queries with Django ORM,

  • There is no support to prefetch related models to increase performance
  • Query results need to be cached manually

Change History (25)

comment:1 by Adnan Umer, 7 years ago

Owner: changed from nobody to Adnan Umer
Status: newassigned

comment:2 by Tim Graham, 7 years ago

Has patch: unset
Summary: Prefetch Related Support in RawQuerySetAdd prefetch related support to RawQuerySet
Triage Stage: UnreviewedAccepted

Tentatively accepting, although I haven't looked into the history to understand if there's a design reason why it's not implemented. A post on django-users suggests that using prefetch_related_objects() (which has since been documented as a public API) works.

I'm unchecking "Has patch" as I don't see a patch anywhere. If there's a patch somewhere, please provide a link.

comment:3 by Adnan Umer, 7 years ago

Has patch: set
Needs tests: set

Here is the link to PR that enabled this feature.

Last edited 7 years ago by Adnan Umer (previous) (diff)

comment:4 by Дилян Палаузов, 7 years ago

Would you mind implementing the same for bulk_create() #28692?

comment:5 by Дилян Палаузов, 7 years ago

Cc: Дилян Палаузов added

in reply to:  4 comment:6 by Adnan Umer, 7 years ago

Replying to Дилян Палаузов:

Would you mind implementing the same for bulk_create() #28692?

Once someone accept the ticket, I'll post commit solution of that. BTW you can temporarily avoid that situation this way

models = ModelA.objects.bulk_create([ModelA(...), ModelA(...), ModelA(...)])
from django.db.models import prefetch_related_objects
prefetch_related_objects(models, 'b')
for m in models:
        print(m.b.description)  # No more extra SELECT query here
Last edited 7 years ago by Adnan Umer (previous) (diff)

comment:7 by Asif Saifuddin Auvi, 7 years ago

Needs documentation: set
Patch needs improvement: set

comment:8 by Adnan Umer, 7 years ago

Description: modified (diff)
Needs tests: unset

comment:9 by Simon Charette, 7 years ago

Unless __bool__ and __len__ support is required for prefetch_related to work they should be added/tracked in another PR/ticket.

in reply to:  9 ; comment:10 by Adnan Umer, 7 years ago

Replying to Simon Charette:

Unless __bool__ and __len__ support is required for prefetch_related to work they should be added/tracked in another PR/ticket.

Got that. Moving that to separate PR.

comment:11 by Adnan Umer, 7 years ago

Description: modified (diff)

comment:12 by Adnan Umer, 7 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:13 by Adnan Umer, 7 years ago

Owner: Adnan Umer removed
Status: assignednew

comment:14 by Adnan Umer, 7 years ago

Owner: set to Adnan Umer
Status: newassigned

comment:15 by Tim Graham, 7 years ago

Patch needs improvement: set

comment:16 by Adnan Umer, 7 years ago

Patch needs improvement: unset

comment:17 by Carlton Gibson, 7 years ago

Patch needs improvement: set

comment:18 by Adnan Umer, 7 years ago

Patch needs improvement: unset

Did requested changes. Requesting to review the PR again.

comment:19 by Carlton Gibson, 7 years ago

Needs documentation: set

Patch is looking good. Just needs some adjustments to the documentation changes.

in reply to:  10 ; comment:20 by Carlton Gibson, 7 years ago

Cc: Carlton Gibson added

Replying to Adnan Umer:

Replying to Simon Charette:

Unless __bool__ and __len__ support is required for prefetch_related to work they should be added/tracked in another PR/ticket.

Got that. Moving that to separate PR.

Did that happen? (I can't see another PR by Adnan)

I ask because the warning in docs/db/sql.txt about this will also need updating:

__bool__() and __len__() are not defined in RawQuerySet, and
thus all RawQuerySet instances are considered True. The reason
these methods are not implemented in RawQuerySet is that implementing
them without internal caching would be a performance drawback and adding
such caching would be backward incompatible.

in reply to:  20 comment:21 by Adnan Umer, 7 years ago

Replying to Carlton Gibson:

Did that happen? (I can't see another PR by Adnan)

I havn't yet opened any new PR for that. I was just waiting on approval/merge of my existing ticket. Once that is done I'll create a new PR for that.

I ask because the warning in docs/db/sql.txt about this will also need updating:

Yes, I've created #29337 and assigned that to myself. Not sure should I open PR once this one got merged or before merge

Last edited 7 years ago by Tim Graham (previous) (diff)

comment:22 by Adnan Umer, 7 years ago

Needs documentation: unset

Unchecking "Needs Documentation" to have PR reviewed again after doing requested changes.

comment:23 by Carlton Gibson, 7 years ago

Triage Stage: AcceptedReady for checkin

comment:24 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 534d8d87:

Fixed #28600 -- Added prefetch_related() support to RawQuerySet.

comment:25 by Дилян Палаузов, 7 years ago

Would you mind implementing the same for bulk_create() #28692?

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