Opened 6 years ago

Closed 6 years ago

Last modified 6 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 Changed 6 years ago by Adnan Umer

Owner: changed from nobody to Adnan Umer
Status: newassigned

comment:2 Changed 6 years ago by Tim Graham

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 Changed 6 years ago by Adnan Umer

Has patch: set
Needs tests: set

Here is the link to PR that enabled this feature.

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

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

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

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

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

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

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 6 years ago by Adnan Umer (previous) (diff)

comment:7 Changed 6 years ago by Asif Saifuddin Auvi

Needs documentation: set
Patch needs improvement: set

comment:8 Changed 6 years ago by Adnan Umer

Description: modified (diff)
Needs tests: unset

comment:9 Changed 6 years ago by Simon Charette

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

comment:10 in reply to:  9 ; Changed 6 years ago by 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.

comment:11 Changed 6 years ago by Adnan Umer

Description: modified (diff)

comment:12 Changed 6 years ago by Adnan Umer

Needs documentation: unset
Patch needs improvement: unset

comment:13 Changed 6 years ago by Adnan Umer

Owner: Adnan Umer deleted
Status: assignednew

comment:14 Changed 6 years ago by Adnan Umer

Owner: set to Adnan Umer
Status: newassigned

comment:15 Changed 6 years ago by Tim Graham

Patch needs improvement: set

comment:16 Changed 6 years ago by Adnan Umer

Patch needs improvement: unset

comment:17 Changed 6 years ago by Carlton Gibson

Patch needs improvement: set

comment:18 Changed 6 years ago by Adnan Umer

Patch needs improvement: unset

Did requested changes. Requesting to review the PR again.

comment:19 Changed 6 years ago by Carlton Gibson

Needs documentation: set

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

comment:20 in reply to:  10 ; Changed 6 years ago by Carlton Gibson

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.

comment:21 in reply to:  20 Changed 6 years ago by Adnan Umer

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 6 years ago by Tim Graham (previous) (diff)

comment:22 Changed 6 years ago by Adnan Umer

Needs documentation: unset

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

comment:23 Changed 6 years ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:24 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 534d8d87:

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

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

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

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