Opened 8 months ago

Closed 4 weeks ago

Last modified 4 weeks 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: master
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 8 months ago by Adnan Umer

Owner: changed from nobody to Adnan Umer
Status: newassigned

comment:2 Changed 8 months 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 8 months ago by Adnan Umer

Has patch: set
Needs tests: set

Here is the link to PR that enabled this feature.

Last edited 8 months ago by Adnan Umer (previous) (diff)

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

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

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

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

comment:6 in reply to:  4 Changed 7 months 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 7 months ago by Adnan Umer (previous) (diff)

comment:7 Changed 6 months ago by Asif Saifuddin Auvi

Needs documentation: set
Patch needs improvement: set

comment:8 Changed 6 months ago by Adnan Umer

Description: modified (diff)
Needs tests: unset

comment:9 Changed 6 months 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 months 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 months ago by Adnan Umer

Description: modified (diff)

comment:12 Changed 6 months ago by Adnan Umer

Needs documentation: unset
Patch needs improvement: unset

comment:13 Changed 6 months ago by Adnan Umer

Owner: Adnan Umer deleted
Status: assignednew

comment:14 Changed 6 months ago by Adnan Umer

Owner: set to Adnan Umer
Status: newassigned

comment:15 Changed 2 months ago by Tim Graham

Patch needs improvement: set

comment:16 Changed 2 months ago by Adnan Umer

Patch needs improvement: unset

comment:17 Changed 6 weeks ago by Carlton Gibson

Patch needs improvement: set

comment:18 Changed 5 weeks ago by Adnan Umer

Patch needs improvement: unset

Did requested changes. Requesting to review the PR again.

comment:19 Changed 5 weeks 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 5 weeks 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 5 weeks 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 4 weeks ago by Tim Graham (previous) (diff)

comment:22 Changed 5 weeks ago by Adnan Umer

Needs documentation: unset

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

comment:23 Changed 5 weeks ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:24 Changed 4 weeks ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 534d8d87:

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

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

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

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