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

Allows to pass queryset with select_related #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allows to pass queryset with select_related #2

wants to merge 1 commit into from

Conversation

ad-m
Copy link
Contributor

@ad-m ad-m commented Feb 7, 2020

No description provided.

Copy link
Owner

@ross ross left a comment

Choose a reason for hiding this comment

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

Can you elaborate a bit on what you ran into/are trying to do here. It's been a LONG time since I've been in this code and my reading of it now it's not clear why the filter needs to be removed. Iirc only _field is involved with the token and that section is looking for the reverse (go back to the previous page) token so selecting only the column involved in the token is sufficient, the rest of the object isn't useful. Again been several years so may be mis-remembering.

@ad-m
Copy link
Contributor Author

ad-m commented Feb 8, 2020

I would like pass Model.objects.select_related('xxx').all() to paginator. Unfortunatly, you can not use select_related and only in single query. Here is example stacktrace:

>>> Case.objects.select_related('monitoring').only('pk').all()
Traceback (most recent call last):
  File "/usr/lib/python3.6/code.py", line 91, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/query.py", line 250, in __repr__
    data = list(self[:REPR_OUTPUT_SIZE + 1])
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/query.py", line 274, in __iter__
    self._fetch_all()
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/query.py", line 1242, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/query.py", line 55, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1120, in execute_sql
    sql, params = self.as_sql()
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 474, in as_sql
    extra_select, order_by, group_by = self.pre_sql_setup()
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 54, in pre_sql_setup
    self.setup_query()
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 45, in setup_query
    self.select, self.klass_info, self.annotation_col_map = self.get_select()
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 240, in get_select
    related_klass_infos = self.get_related_selections(select)
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 830, in get_related_selections
    only_load.get(field_model)):
  File "/opt/feder/releases/releases/20200207050920/env/lib/python3.6/site-packages/django/db/models/query_utils.py", line 253, in select_related_descend
    (field.model._meta.object_name, field.name))
django.db.models.query_utils.InvalidQuery: Field Case.monitoring cannot be both deferred and traversed using select_related at the same time.

@ross
Copy link
Owner

ross commented Feb 10, 2020

I would like pass Model.objects.select_related('xxx').all() to paginator. Unfortunatly, you can not use select_related and only in single query.

Hrm. That's an interesting case. The reason that's there is to avoid having to fully load/serialize the previous page's objects, instead just grabbing the field their paginated with. Removing that only will cause them to all be loaded up and deserialized, and if using a select_related all of the relations will be as well, just to grab the offset token for the previous page. None of which will be used.

To avoid the extra overhead/hurting the performance of the common/existing case I believe it'd be preferable to add in an extra parameter to the constructor, select_related that gets added to the qs here just when building the object_list and doesn't get applied when finding the previous page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants