Skip to content

Hyperlinked PK optimization. #2242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 10, 2014
Merged

Conversation

tomchristie
Copy link
Member

Closes #1872.

@tomchristie tomchristie added this to the 3.0.1 Release milestone Dec 9, 2014
@@ -197,7 +199,8 @@ def test_foreign_key_retrieve(self):
{'url': 'http://testserver/foreignkeysource/2/', 'name': 'source-2', 'target': 'http://testserver/foreignkeytarget/1/'},
{'url': 'http://testserver/foreignkeysource/3/', 'name': 'source-3', 'target': 'http://testserver/foreignkeytarget/1/'}
]
self.assertEqual(serializer.data, expected)
with self.assertNumQueries(1):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this fix, this is 4.

@carljm
Copy link
Contributor

carljm commented Dec 9, 2014

This looks reasonable to me! Definitely cleaner than the existing workarounds in PrimaryKeyRelatedField, and extends them to HyperlinkedRelatedField as well.

@jpadilla
Copy link
Member

jpadilla commented Dec 9, 2014

👍

@tomchristie
Copy link
Member Author

TODO: Confirm interaction between .value_list and .select_related - don't want to stomp on anyone's optimizations. Possibly also consider .only if that works more seamlessly. Consider same set of improvements for non-pk values.

@tomchristie
Copy link
Member Author

Also possible - drop the .values_list bit until later and just have the main pk lookup improvement in for 3.0.1.

@tomchristie tomchristie mentioned this pull request Dec 10, 2014
5 tasks
@tomchristie
Copy link
Member Author

The .values_list for iterables doesn't work as it removes the benefits of any select_related or prefetch_related.

Prefetch objects in Django 1.7 might let us solve this. See:

https://docs.djangoproject.com/en/1.7/ref/models/queries/#prefetch-objects
https://code.djangoproject.com/ticket/17001
https://code.djangoproject.com/ticket/20923

This seems good to go now tho.

tomchristie added a commit that referenced this pull request Dec 10, 2014
@tomchristie tomchristie merged commit 313c36f into master Dec 10, 2014
@tomchristie tomchristie deleted the hyperlinked-pk-optimization branch December 10, 2014 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize pk-based hyperlinked relationships.
3 participants