Skip to content
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

Suggestion: make query impact queryset to reduce django sql selection #237

Open
fturhan opened this issue May 30, 2021 · 8 comments
Open
Labels
discussion Discussion about implementation details enhancement New feature or request

Comments

@fturhan
Copy link

fturhan commented May 30, 2021

Hi, i'm using this module for a while and i thinks it would be awesome if querys can impact the django queryset selection (with only and/or defer) to significantly reduce the amount of information being selected. But i do not know how to achieve this. Maybe this can be a feature to add to the module.

@yezyilomo
Copy link
Owner

I'm not sure if I have understood what you described, but isn't it what's performed by EagerLoadingMixin?.

@fturhan
Copy link
Author

fturhan commented Jun 1, 2021

I'm currently using Django debug panel and when i use restql to reduce the amount of information, i can see that i fetch the whole objects (nested objects too) in the sql part, however i only need some precise informations.

@yezyilomo
Copy link
Owner

Are you using EagerLoadingMixin in your views?.

@fturhan
Copy link
Author

fturhan commented Jun 1, 2021

Yes i'm using it

@yezyilomo
Copy link
Owner

yezyilomo commented Jun 2, 2021

I’ve never tested this lib on Django debug panel but tests suggests otherwise take this below as an example

def test_list_eager_loading_view_mixin(self):
"""
Ensure that we apply our prefetching or joins when we explicitly ask for fields in the
mapping.
"""
non_mixin_url = reverse_lazy("student-list")
mixin_url = reverse_lazy("student_eager_loading-list")
self.add_second_student()
# This fetches both course names and books for each, so 5 queries total.
with self.assertNumQueries(5):
response = self.client.get(non_mixin_url + '?query={name, age, course{name, books}}', format="json")
self.assertEqual(
response.data,
[
{
"name": "Yezy",
"age": 24,
"course": {
"name": "Data Structures",
"books": [
{"title": "Advanced Data Structures", "author": "S.Mobit"},
{"title": "Basic Data Structures", "author": "S.Mobit"}
]
}
},
{
"name": "Tyler",
"age": 25,
"course": {
"name": "Algorithms",
"books": [
{"title": "Algorithm Design", "author": "S.Mobit"},
{"title": "Proving Algorithms", "author": "S.Mobit"}
]
}
},
]
)
# This fetches the course information with the student and prefetches once for the books.
with self.assertNumQueries(2):
response = self.client.get(mixin_url + '?query={name, age, program{name, books}}', format="json")
self.assertEqual(
response.data,
[
{
"name": "Yezy",
"age": 24,
"program": {
"name": "Data Structures",
"books": [
{"title": "Advanced Data Structures", "author": "S.Mobit"},
{"title": "Basic Data Structures", "author": "S.Mobit"}
]
}
},
{
"name": "Tyler",
"age": 25,
"program": {
"name": "Algorithms",
"books": [
{"title": "Algorithm Design", "author": "S.Mobit"},
{"title": "Proving Algorithms", "author": "S.Mobit"}
]
}
},
]
)

You can see there that the number of queries due to nested fields went from 5 to 2 after using EagerLoadingMixin.

@fturhan
Copy link
Author

fturhan commented Jun 2, 2021

I'm sorry but i have some difficulties to explain my issue. for example when i query {course{books{title}} and then i check what was fetch in the sql part, i found out that all fields were fetch

@yezyilomo
Copy link
Owner

for example when i query {course{books{title}} and then i check what was fetch in the sql part, i found out that all fields were fetch

Ooh!, now I got you. Djang RESTQL doesn't support this for now, but since you brought it we might think of a way to implement it, I think it could help boosting the speed of database queries. I know one way to make ORM select specific fields is applying .only([field1, field2, ...]) on your queryset, since the list of fields selected is dynamic we could find a way to pass them before serialization is done.

@yezyilomo yezyilomo added discussion Discussion about implementation details enhancement New feature or request labels Jun 2, 2021
@resurrexi
Copy link
Contributor

This would be a nice addition, but I can foresee issues where there is no underlying field in the database table, even though it is represented in the serializer such as a SerializerMethodField.

Now that I think about this suggestion, I wonder if Django RESTQL limits the querying of fields if there is an explicit fields argument passed into a NestedField, e.g:

work_order = NestedField(
    WorkOrderSerializer,
    read_only=True,
    fields=["work_order_id"]  # will the query planner only pull `work_order_id`?
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion about implementation details enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants