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

SQL queries are duplicated when mixing in with MultipleObjectMixin-based views e.g. django-filter #914

Open
mmirate opened this issue Apr 17, 2023 · 0 comments

Comments

@mmirate
Copy link

mmirate commented Apr 17, 2023

FilterView from django-filter is a subclass of the builtin Django list view (or MultipleObjectMixin). While the get_queryset-based interoperability is good, there is one slight problem with having a class-based view that subclasses from SingleTableMixin and FilterView.

The usual, actually-used, responsive to table sorting, correct call stack for fetching the table's data, looks something like this:

/home/$USER/$REPO/$PKG/apps/$APP/filters.py in get(201) # the actual View subclass
  return super().get(request, *args, **kwargs)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_filters/views.py in get(85)
  context = self.get_context_data(filter=self.filterset,

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/views.py in get_context_data(161)
  table = self.get_table(**self.get_table_kwargs())

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/views.py in get_table(123)
  return RequestConfig(self.request, paginate=self.get_table_pagination(table)).configure(

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/config.py in configure(61)
  table.paginate(**kwargs)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/tables.py in paginate(583)
  self.page = self.paginator.page(page)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/paginators.py in page(85)
  objects = list(self.object_list[bottom : top + self.orphans + look_ahead_items])

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/rows.py in __len__(325)
  length = len(self.data)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django/db/models/query.py in __len__(262)
  self._fetch_all()

Note particularly the presence of the second line of the body of

def get_context_data(self, **kwargs: Any) -> Dict[str, Any]:
"""
Overridden version of `.TemplateResponseMixin` to inject the table into
the template's context.
"""
context = super().get_context_data(**kwargs)
table = self.get_table(**self.get_table_kwargs())
context[self.get_context_table_name(table)] = table
return context

However, with this inheritance hierarchy, there is a second route to the database, which produces a useless not-properly-sorted duplicate of the data and places it in the template context where, because the data is being shown in a table, it will not be used, wasting effort on the server and time on the webpage load.

/home/$USER/$REPO/$PKG/apps/$APP/filters.py in get(201)
  return super().get(request, *args, **kwargs)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_filters/views.py in get(85)
  context = self.get_context_data(filter=self.filterset,

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/views.py in get_context_data(160) # not 161! big difference
  context = super().get_context_data(**kwargs)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django/views/generic/list.py in get_context_data(119)
  paginator, page, queryset, is_paginated = self.paginate_queryset(queryset, page_size)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django/views/generic/list.py in paginate_queryset(69)
  page = paginator.page(page_number)

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django_tables2/paginators.py in page(85)
  objects = list(self.object_list[bottom : top + self.orphans + look_ahead_items])

/home/$USER/.virtualenvs/$VENV/lib/python3.10/site-packages/django/db/models/query.py in __iter__(280)
  self._fetch_all()

In other words, when inheriting multiply from SingleTableMixin and FilterView, that super() call in SingleTableMixin.get_context_data goes to Django's builtin MultipleObjectMixin.


Since I know that my codebase is only using SingleTableMixin together with FilterView, I can monkeypatch SingleTableMixin.get_context_data so that it instead calls super(MultipleObjectMixin, self) so as to only consider classes after MultipleObjectMixin in the MRO - in this case, ContextMixin which is the only other get_context_data implementation in the whole MRO after SingleTableMixin.

However, that solution will not work in the general case that someone else is mixing SingleTableMixin into a view not derived from MultipleObjectMixin.

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

No branches or pull requests

1 participant