Enhance queryset with prefetch for many-to-many fields#32
Conversation
Added prefetching of many-to-many fields in queryset.
There was a problem hiding this comment.
Pull request overview
This PR enhances the queryset optimization in get_queryset() to automatically detect and prefetch many-to-many fields alongside the existing select_related optimization for foreign keys. This prevents N+1 query problems when accessing many-to-many relationships through nested field notation.
Key Changes:
- Added
prefetch_fieldslist to collect ManyToManyField relationships - Updated field type detection logic to differentiate between ForeignKey/OneToOneField and ManyToManyField
- Applied
prefetch_related()to the queryset when many-to-many fields are detected
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # This is a related field | ||
| base_field = field.split("__")[0] | ||
| if base_field not in select_related_fields: | ||
| model_field = self.model._meta.get_field(base_field) |
There was a problem hiding this comment.
The models module is not imported at the module level. This will cause a NameError when this code executes. Add from django.db import models at the top of the file with the other imports.
There was a problem hiding this comment.
@copilot that's wrong because this is a class extending from another and as you can see it is internal as parameter not from that value
| if prefetch_fields: | ||
| queryset = queryset.prefetch_related(*prefetch_fields) |
There was a problem hiding this comment.
The new prefetch_related functionality for ManyToManyField relationships lacks test coverage. Consider adding tests that verify:
- ManyToMany fields are correctly identified and added to prefetch_fields
- The queryset actually has prefetch_related applied
- No N+1 query issues occur when accessing many-to-many relationships
You may need to add a ManyToManyField to one of the test models (e.g., in tests/test_app/models.py) to properly test this functionality.
| model_field = self.model._meta.get_field(base_field) | ||
| if isinstance(model_field, (models.ForeignKey, models.OneToOneField)): | ||
| select_related_fields.append(base_field) | ||
| if isinstance(model_field, models.ManyToManyField): | ||
| prefetch_fields.append(base_field) |
There was a problem hiding this comment.
The call to self.model._meta.get_field(base_field) can raise a FieldDoesNotExist exception if the base_field doesn't exist on the model. Consider wrapping this in a try-except block to handle invalid field names gracefully:
try:
model_field = self.model._meta.get_field(base_field)
if isinstance(model_field, (models.ForeignKey, models.OneToOneField)):
# ...
except FieldDoesNotExist:
continue # Skip invalid field namesThis pattern is used elsewhere in the codebase (e.g., in turbodrf/mixins.py line 153).
There was a problem hiding this comment.
base_field always exists, @copilot as you can see in the lines above
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thanks for this PR! M2M field prefetching has been implemented in v0.2.0 - the get_queryset() method now automatically detects and applies prefetch_related() for many-to-many relationships alongside the existing select_related() optimization for foreign keys. |
Added prefetching of many-to-many fields in queryset.