Skip to content

Commit cac09bd

Browse files
slivercn2ygk
authored andcommitted
Remove not always supported auto select_related (#651)
Polymorphic models do not always support `select_related` as of https://django-polymorphic.readthedocs.io/en/stable/advanced.html?highlight=select_related#about-queryset-methods and might actually be slower in some cases as explained in https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html?highlight=usage#performance-improvements Hence reverting change of AutoPreloadMixin as it has not been released yet.
1 parent b884535 commit cac09bd

File tree

4 files changed

+9
-34
lines changed

4 files changed

+9
-34
lines changed

CHANGELOG.md

-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ any parts of the framework not mentioned in the documentation should generally b
1717
### Changed
1818

1919
* Allow to define `select_related` per include using [select_for_includes](https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#performance-improvements)
20-
* Reduce number of queries to calculate includes by using `select_related` when possible
2120
* Use REST framework serializer functionality to extract includes. This adds support like using
2221
dotted notations in source attribute in `ResourceRelatedField`.
2322

@@ -31,7 +30,6 @@ any parts of the framework not mentioned in the documentation should generally b
3130
### Deprecated
3231

3332
* Deprecate `PrefetchForIncludesHelperMixin` use `PreloadIncludesMixin` instead
34-
* Deprecate `AutoPrefetchMixin` use `AutoPreloadMixin` instead
3533

3634
## [2.7.0] - 2019-01-14
3735

example/tests/test_performance.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def test_query_count_include_author(self):
5353
4. Author types prefetched
5454
5. Entries prefetched
5555
"""
56-
with self.assertNumQueries(4):
56+
with self.assertNumQueries(5):
5757
response = self.client.get('/comments?include=author&page[size]=25')
5858
self.assertEqual(len(response.data['results']), 25)
5959

example/views.py

+2-5
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from rest_framework_json_api.filters import OrderingFilter, QueryParameterValidationFilter
1313
from rest_framework_json_api.pagination import JsonApiPageNumberPagination
1414
from rest_framework_json_api.utils import format_drf_errors
15-
from rest_framework_json_api.views import ModelViewSet, PreloadIncludesMixin, RelationshipView
15+
from rest_framework_json_api.views import ModelViewSet, RelationshipView
1616

1717
from example.models import Author, Blog, Comment, Company, Entry, Project, ProjectType
1818
from example.serializers import (
@@ -200,12 +200,9 @@ def get_queryset(self, *args, **kwargs):
200200
return super(CommentViewSet, self).get_queryset()
201201

202202

203-
class CompanyViewset(PreloadIncludesMixin, viewsets.ModelViewSet):
203+
class CompanyViewset(ModelViewSet):
204204
queryset = Company.objects.all()
205205
serializer_class = CompanySerializer
206-
prefetch_for_includes = {
207-
'current_project': ['current_project'],
208-
}
209206

210207

211208
class ProjectViewset(ModelViewSet):

rest_framework_json_api/views.py

+6-26
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,18 @@ def get_queryset(self, *args, **kwargs):
115115
return qs
116116

117117

118-
class AutoPreloadMixin(object):
119-
118+
class AutoPrefetchMixin(object):
120119
def get_queryset(self, *args, **kwargs):
121120
""" This mixin adds automatic prefetching for OneToOne and ManyToMany fields. """
122-
qs = super(AutoPreloadMixin, self).get_queryset(*args, **kwargs)
121+
qs = super(AutoPrefetchMixin, self).get_queryset(*args, **kwargs)
122+
123123
included_resources = get_included_resources(self.request)
124124

125125
for included in included_resources + ['__all__']:
126126
# If include was not defined, trying to resolve it automatically
127127
included_model = None
128128
levels = included.split('.')
129129
level_model = qs.model
130-
# Suppose we can do select_related by default
131-
can_select_related = True
132130
for level in levels:
133131
if not hasattr(level_model, level):
134132
break
@@ -144,12 +142,6 @@ def get_queryset(self, *args, **kwargs):
144142
if not (is_forward_relation or is_reverse_relation):
145143
break
146144

147-
# Figuring out if relation should be select related rather than prefetch_related
148-
# If at least one relation in the chain is not "selectable" then use "prefetch"
149-
can_select_related &= (
150-
issubclass(field_class, (ForwardManyToOneDescriptor, ReverseOneToOneDescriptor))
151-
)
152-
153145
if level == levels[-1]:
154146
included_model = field
155147
else:
@@ -165,23 +157,11 @@ def get_queryset(self, *args, **kwargs):
165157
level_model = model_field.model
166158

167159
if included_model is not None:
168-
if can_select_related:
169-
qs = qs.select_related(included.replace('.', '__'))
170-
else:
171-
qs = qs.prefetch_related(included.replace('.', '__'))
160+
qs = qs.prefetch_related(included.replace('.', '__'))
172161

173162
return qs
174163

175164

176-
class AutoPrefetchMixin(AutoPreloadMixin):
177-
178-
def __init__(self, *args, **kwargs):
179-
warnings.warn("AutoPrefetchMixin is deprecated. "
180-
"Use AutoPreloadMixin instead",
181-
DeprecationWarning)
182-
super(AutoPrefetchMixin, self).__init__(*args, **kwargs)
183-
184-
185165
class RelatedMixin(object):
186166
"""
187167
This mixin handles all related entities, whose Serializers are declared in "related_serializers"
@@ -259,14 +239,14 @@ def get_related_instance(self):
259239
raise NotFound
260240

261241

262-
class ModelViewSet(AutoPreloadMixin,
242+
class ModelViewSet(AutoPrefetchMixin,
263243
PreloadIncludesMixin,
264244
RelatedMixin,
265245
viewsets.ModelViewSet):
266246
pass
267247

268248

269-
class ReadOnlyModelViewSet(AutoPreloadMixin,
249+
class ReadOnlyModelViewSet(AutoPrefetchMixin,
270250
RelatedMixin,
271251
viewsets.ReadOnlyModelViewSet):
272252
pass

0 commit comments

Comments
 (0)