From 720a37d3dedc501968bebaca3a339c72392b9c81 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 9 Dec 2014 17:28:56 +0000 Subject: Hyperlinked PK optimization. Closes #1872. --- rest_framework/relations.py | 64 ++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 27 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 75d68204..ae4086ec 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -84,9 +84,34 @@ class RelatedField(Field): queryset = queryset.all() return queryset + def use_pk_only_optimization(self): + return False + + def get_attribute(self, instance): + if self.use_pk_only_optimization(): + try: + # Optimized case, return a mock object only containing the pk attribute. + instance = get_attribute(instance, self.source_attrs[:-1]) + return PKOnlyObject(pk=instance.serializable_value(self.source_attrs[-1])) + except AttributeError: + pass + + # Standard case, return the object instance. + return get_attribute(instance, self.source_attrs) + def get_iterable(self, instance, source_attrs): relationship = get_attribute(instance, source_attrs) - return relationship.all() if (hasattr(relationship, 'all')) else relationship + relationship = relationship.all() if (hasattr(relationship, 'all')) else relationship + + if self.use_pk_only_optimization(): + # Optimized case, return mock objects only containing the pk attribute. + return [ + PKOnlyObject(pk=pk) + for pk in relationship.values_list('pk', flat=True) + ] + + # Standard case, return the object instances. + return relationship @property def choices(self): @@ -120,6 +145,9 @@ class PrimaryKeyRelatedField(RelatedField): 'incorrect_type': _('Incorrect type. Expected pk value, received {data_type}.'), } + def use_pk_only_optimization(self): + return True + def to_internal_value(self, data): try: return self.get_queryset().get(pk=data) @@ -128,32 +156,6 @@ class PrimaryKeyRelatedField(RelatedField): except (TypeError, ValueError): self.fail('incorrect_type', data_type=type(data).__name__) - def get_attribute(self, instance): - # We customize `get_attribute` here for performance reasons. - # For relationships the instance will already have the pk of - # the related object. We return this directly instead of returning the - # object itself, which would require a database lookup. - try: - instance = get_attribute(instance, self.source_attrs[:-1]) - return PKOnlyObject(pk=instance.serializable_value(self.source_attrs[-1])) - except AttributeError: - return get_attribute(instance, self.source_attrs) - - def get_iterable(self, instance, source_attrs): - # For consistency with `get_attribute` we're using `serializable_value()` - # here. Typically there won't be any difference, but some custom field - # types might return a non-primitive value for the pk otherwise. - # - # We could try to get smart with `values_list('pk', flat=True)`, which - # would be better in some case, but would actually end up with *more* - # queries if the developer is using `prefetch_related` across the - # relationship. - relationship = super(PrimaryKeyRelatedField, self).get_iterable(instance, source_attrs) - return [ - PKOnlyObject(pk=item.serializable_value('pk')) - for item in relationship - ] - def to_representation(self, value): return value.pk @@ -184,6 +186,9 @@ class HyperlinkedRelatedField(RelatedField): super(HyperlinkedRelatedField, self).__init__(**kwargs) + def use_pk_only_optimization(self): + return self.lookup_field == 'pk' + def get_object(self, view_name, view_args, view_kwargs): """ Return the object corresponding to a matched URL. @@ -285,6 +290,11 @@ class HyperlinkedIdentityField(HyperlinkedRelatedField): kwargs['source'] = '*' super(HyperlinkedIdentityField, self).__init__(view_name, **kwargs) + def use_pk_only_optimization(self): + # We have the complete object instance already. We don't need + # to run the 'only get the pk for this relationship' code. + return False + class SlugRelatedField(RelatedField): """ -- cgit v1.2.3 From ca7b1f6d5189398be8a0d24b1e01577281b1b187 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 10 Dec 2014 21:09:45 +0000 Subject: Optimizations play nicely with select_related, prefetch_related --- rest_framework/relations.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/relations.py b/rest_framework/relations.py index ae4086ec..6b693e4b 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -101,17 +101,7 @@ class RelatedField(Field): def get_iterable(self, instance, source_attrs): relationship = get_attribute(instance, source_attrs) - relationship = relationship.all() if (hasattr(relationship, 'all')) else relationship - - if self.use_pk_only_optimization(): - # Optimized case, return mock objects only containing the pk attribute. - return [ - PKOnlyObject(pk=pk) - for pk in relationship.values_list('pk', flat=True) - ] - - # Standard case, return the object instances. - return relationship + return relationship.all() if (hasattr(relationship, 'all')) else relationship @property def choices(self): -- cgit v1.2.3 From 9d3810f313123ce48dea48f3c3f0f2cae5816bde Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 10 Dec 2014 22:09:24 +0000 Subject: Drop get_iterable() --- rest_framework/relations.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 6b693e4b..a5f97596 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -88,7 +88,7 @@ class RelatedField(Field): return False def get_attribute(self, instance): - if self.use_pk_only_optimization(): + if self.use_pk_only_optimization() and self.source_attrs: try: # Optimized case, return a mock object only containing the pk attribute. instance = get_attribute(instance, self.source_attrs[:-1]) @@ -99,10 +99,6 @@ class RelatedField(Field): # Standard case, return the object instance. return get_attribute(instance, self.source_attrs) - def get_iterable(self, instance, source_attrs): - relationship = get_attribute(instance, source_attrs) - return relationship.all() if (hasattr(relationship, 'all')) else relationship - @property def choices(self): return dict([ @@ -349,7 +345,8 @@ class ManyRelatedField(Field): ] def get_attribute(self, instance): - return self.child_relation.get_iterable(instance, self.source_attrs) + relationship = get_attribute(instance, self.source_attrs) + return relationship.all() if (hasattr(relationship, 'all')) else relationship def to_representation(self, iterable): return [ -- cgit v1.2.3 From 1e336ef30da9f7366fc68185016c4011c25e1199 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 10 Dec 2014 22:10:45 +0000 Subject: Move comment --- rest_framework/relations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rest_framework') diff --git a/rest_framework/relations.py b/rest_framework/relations.py index a5f97596..892ce6c1 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -89,8 +89,8 @@ class RelatedField(Field): def get_attribute(self, instance): if self.use_pk_only_optimization() and self.source_attrs: + # Optimized case, return a mock object only containing the pk attribute. try: - # Optimized case, return a mock object only containing the pk attribute. instance = get_attribute(instance, self.source_attrs[:-1]) return PKOnlyObject(pk=instance.serializable_value(self.source_attrs[-1])) except AttributeError: -- cgit v1.2.3