Add smarter datagrid pagination, query augmentation, and distinct handling.

Review Request #13367 — Created Oct. 22, 2023 and submitted

Information

Djblets
release-3.x

Reviewers

Datagrids have historically used the same queryset for both page data
and for pagination. This led to expensive pagination detection, as the
nature of the paginator and Django's ORM results in the total page count
be computed as a SELECT COUNT(*) FROM (<subquery>), which is very
slow and gets worse the more data is matched.

To resolve this, we're now using dual querysets. One queryset is
responsible purely for the filtering. The other builds on this and is
responsible for the page data.

We use a new DataGridPaginator class to handle this, since Django's
paginator doesn't give us any way to set the total item count. This
class gives us that ability. It's now deprecated for subclasses to
return a paginator instance that is not a subclass of
DataGridPaginator, but for the moment, it's still supported.

The old way of augmenting querysets through the datagrid or columns is
also deprecated, as it was never built for this dual queryset method in
mind. Instead, we have two new augmentation pathways: One for filtering,
one for data collection. The filtering path is used for both querysets,
and the data collection path is only used for the page data queryset.
We'll be able to make further use of this later on when proper support
for filtering arrives.

And last, but not least, querysets are no longer forced into a
DISTINCT mode. This is still on by default, but datagrids can turn it
off. When on, the database resolves duplicates, but at a higher cost. If
a queryset is written well, it can avoid duplicates without the need for
this. Future versions of Djblets may switch this off by default.

Unit tests pass in Djblets and Review Board.

Made use of the new support to improve queryset counts in Review Board.

Summary ID
Add smarter datagrid pagination, query augmentation, and distinct handling.
Datagrids have historically used the same queryset for both page data and for pagination. This led to expensive pagination detection, as the nature of the paginator and Django's ORM results in the total page count be computed as a `SELECT COUNT(*) FROM (<subquery>)`, which is very slow and gets worse the more data is matched. To resolve this, we're now using dual querysets. One queryset is responsible purely for the filtering. The other builds on this and is responsible for the page data. We use a new `DataGridPaginator` class to handle this, since Django's paginator doesn't give us any way to set the total item count. This class gives us that ability. It's now deprecated for subclasses to return a paginator instance that is not a subclass of `DataGridPaginator`, but for the moment, it's still supported. The old way of augmenting querysets through the datagrid or columns is also deprecated, as it was never built for this dual queryset method in mind. Instead, we have two new augmentation pathways: One for filtering, one for data collection. The filtering path is used for both querysets, and the data collection path is only used for the page data queryset. We'll be able to make further use of this later on when proper support for filtering arrives. And last, but not least, querysets are no longer forced into a `DISTINCT` mode. This is still on by default, but datagrids can turn it off. When on, the database resolves duplicates, but at a higher cost. If a queryset is written well, it can avoid duplicates without the need for this. Future versions of Djblets may switch this off by default.
c61e887873985749850cff3758aa2875ab282e4c
Description From Last Updated

Could add an # Instance Variables # between these.

maubinmaubin

Missing docs for state, request and **kwargs.

maubinmaubin

Missing docs for state, request and **kwargs.

maubinmaubin

Could add a "Version Changed" for the use_distinct arg.

maubinmaubin

Missing docs for **kwargs.

maubinmaubin

Missing docs for **kwargs.

maubinmaubin
david
  1. Ship It!
  2. 
      
maubin
  1. 
      
  2. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Could add an # Instance Variables # between these.

  3. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
    Show all issues

    Missing docs for state, request and **kwargs.

  4. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
    Show all issues

    Missing docs for state, request and **kwargs.

  5. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues

    Could add a "Version Changed" for the use_distinct arg.

  6. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
    Show all issues

    Missing docs for **kwargs.

  7. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
    Show all issues

    Missing docs for **kwargs.

  8. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.x (8560908)
Loading...