jieter / django-tables2

django-tables2 - An app for creating HTML tables
https://django-tables2.readthedocs.io/en/latest/
Other
1.86k stars 426 forks source link

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

Open mmirate opened 1 year ago

mmirate commented 1 year ago

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 https://github.com/jieter/django-tables2/blob/7a5a1fbf2bb5ce2ebe48e962b00c8ff327fea644/django_tables2/views.py#L155-L163

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.