Support multiple fields; Benchmarks (#8)

Author: nitely

.coveragerc

@@ -1,4 +1,5 @@
 [report]
 omit =
+    runbench.py
     runtests.py
     setup.py

.gitignore

@@ -1,3 +1,6 @@
+/mysite
+/manage.py
+
 # Byte-compiled / optimized / DLL files
 __pycache__/
 *.py[cod]
@@ -58,4 +61,4 @@ docs/_build/
 # Vim
 *~
 *.swp
-*.swo
+*.swo

CHANGES.md

@@ -1,3 +1,8 @@
+1.2.0
+==================
+
+* Support paginating by multiple fields
+
 1.1.0
 ==================
 

Makefile

@@ -13,4 +13,16 @@ sdist: test clean
 release: sdist
 	twine upload dist/*
 
+bench_build:
+	cd bench && docker-compose build
+
+bench_clean:
+	cd bench \
+	&& docker-compose stop \
+	&& docker-compose rm --force -v
+
+bench_run:
+	cd bench \
+	&& docker-compose run --rm --entrypoint '/bin/sh -c' paginator '/bin/sh'
+
 .PHONY: clean docs test sdist release

README.md

@@ -22,13 +22,13 @@ based on the top/last keyset
 This approach has two main advantages over the *OFFSET/LIMIT* approach:
 
 * is correct: unlike the *offset/limit* based approach it correctly handles
-new entries and deleted entries. Last row of Page 4 does not show up as first
-row of Page 5 just because row 23 on Page 2 was deleted in the meantime.
-Nor do rows mysteriously vanish between pages. These anomalies are common
-with the *offset/limit* based approach, but the *keyset* based solution does
-a much better job at avoiding them.
+  new entries and deleted entries. Last row of Page 4 does not show up as first
+  row of Page 5 just because row 23 on Page 2 was deleted in the meantime.
+  Nor do rows mysteriously vanish between pages. These anomalies are common
+  with the *offset/limit* based approach, but the *keyset* based solution does
+  a much better job at avoiding them.
 * is fast: all operations can be solved with a fast row positioning followed
-by a range scan in the desired direction.
+  by a range scan in the desired direction.
 
 For a full explanation go to
 [the seek method](http://use-the-index-luke.com/sql/partial-results/fetch-next-page)
@@ -46,11 +46,6 @@ infinite-scroll-pagination requires the following software to be installed:
 pip install django-infinite-scroll-pagination
 ```
 
-## Django Rest Framework (DRF)
-
-DRF has the built-in `CursorPagination`
-that is similar to this lib. Use that instead.
-
 ## Usage
 
 This example paginates by a `created_at` date field:
@@ -102,12 +97,24 @@ def pagination_ajax(request):
     return HttpResponse(json.dumps(data), content_type="application/json")
 ```
 
-Paginating by pk, id or some `unique=True` field:
+Paginating by `pk`, `id`, or some `unique=True` field:
 
 ```python
 page = paginator.paginate(queryset, lookup_field='pk', value=pk, per_page=20)
 ```
 
+Paginating by multiple fields:
+
+```python
+page = paginator.paginate(
+    queryset,
+    lookup_field=('-is_pinned', '-created_at', '-pk'),
+    value=(is_pinned, created_at, pk),
+    per_page=20)
+```
+
+> Make sure the last field is `unique=True`, or `pk`
+
 ## Items order
 
 DESC order:
@@ -178,14 +185,23 @@ class Article(models.Model):
 
     class Meta:
         indexes = [
-            models.Index(fields=['created_at', 'pk'],
-            models.Index(fields=['-created_at', '-pk'])]
+            models.Index(fields=['created_at', 'id']),
+            models.Index(fields=['-created_at', '-id'])]
 ```
 
 > Note: an index is require for both directions,
   since the query has a `LIMIT`.
   See [indexes-ordering](https://www.postgresql.org/docs/9.3/indexes-ordering.html)
 
+However, this library does not implements the fast "row values"
+variant of [the seek method](https://use-the-index-luke.com/sql/partial-results/fetch-next-page).
+What this means is the index is only
+used on the first field. If the first field is a boolean,
+then it won't be used. So, it's pointless to index anything other than the first field.
+See [PR #8](https://github.com/nitely/django-infinite-scroll-pagination/pull/8)
+if you are interested in benchmarks numbers, and please let me know
+if there is a way to implement the "row values" variant without using raw SQL.
+
 Pass a limit to the following methods,
 or use them in places where there won't be
 many records, otherwise they get expensive fast:

bench/Dockerfile

@@ -0,0 +1,23 @@
+FROM python:3.7.2-alpine
+
+ENV PYTHONDONTWRITEBYTECODE 1
+ENV PYTHONUNBUFFERED 1
+
+RUN apk update \
+    && apk add  \
+        postgresql-client \
+        postgresql-dev \
+        gcc \
+        musl-dev \
+        make \
+        libffi-dev
+
+RUN mkdir -p /usr/src/app
+WORKDIR /usr/src/app
+
+RUN pip install --upgrade pip \
+    && pip install Django==2.2.8 \
+    && pip install psycopg2-binary==2.8.6
+
+CMD until pg_isready --username=postgres --host=database; do sleep 1; done;
+ENTRYPOINT /bin/sh

bench/docker-compose.yml

@@ -0,0 +1,13 @@
+version: '3'
+
+services:
+  database:
+    image: postgres:10.5
+    restart: always
+  paginator:
+    build: .
+    hostname: paginator
+    volumes:
+      - ..:/usr/src/app
+    links:
+      - database

infinite_scroll_pagination/paginator.py

@@ -1,13 +1,12 @@
 #-*- coding: utf-8 -*-
 
-
 try:
     from collections.abc import Sequence
 except ImportError:
     from collections import Sequence
 
 from django.core.paginator import EmptyPage
-from django.db.models import QuerySet
+from django.db.models import QuerySet, Q
 
 __all__ = [
     'SeekPaginator',
@@ -17,8 +16,7 @@
     'PREV_PAGE']
 
 
-NEXT_PAGE = 1
-PREV_PAGE = 2
+NEXT_PAGE, PREV_PAGE, DESC, ASC = range(1, 5)
 
 
 class _NoPk:
@@ -33,52 +31,73 @@ def __repr__(self):
 # the first page
 _NO_PK = _NoPk()
 
+# XXX simplify things by removing the pk parameter,
+#     and requiring it as last field/value; we should
+#     also validate there is a single unique=True field,
+#     and it's the last one; this is a breaking chance, though
 
-class SeekPaginator(object):
 
+class SeekPaginator:
     def __init__(self, query_set, per_page, lookup_field):
         assert isinstance(query_set, QuerySet), 'QuerySet expected'
         assert isinstance(per_page, int), 'Int expected'
-        assert isinstance(lookup_field, str), 'String expected'
+        #assert isinstance(lookup_field, str), 'String expected'
         self.query_set = query_set
         self.per_page = per_page
-        self.is_desc = lookup_field.startswith('-')
-        self.is_asc = not self.is_desc
-        self.lookup_field = lookup_field.lstrip('-')
+        if isinstance(lookup_field, str):
+            lookup_field = (lookup_field,)
+        self.lookup_fields = lookup_field
+
+    @property
+    def fields(self):
+        return tuple(f.lstrip('-') for f in self.lookup_fields)
+
+    @property
+    def fields_direction(self):
+        d = {True: DESC, False: ASC}
+        return tuple(
+            (f.lstrip('-'), d[f.startswith('-')])
+            for f in self.lookup_fields)
 
     def prepare_order(self, has_pk, move_to):
-        pk_sort = 'pk'
-        lookup_sort = self.lookup_field
-        if ((self.is_desc and move_to == NEXT_PAGE) or
-                (self.is_asc and move_to == PREV_PAGE)):
-            pk_sort = '-%s' % pk_sort
-            lookup_sort = '-%s' % lookup_sort
+        fields = list(self.fields_direction)
         if has_pk:
-            return [lookup_sort, pk_sort]
-        return [lookup_sort]
-
-    def prepare_lookup(self, value, pk, move_to):
-        lookup_include = '%s__gt' % self.lookup_field
-        lookup_exclude_pk = 'pk__lte'
-        if ((self.is_desc and move_to == NEXT_PAGE) or
-                (self.is_asc and move_to == PREV_PAGE)):
-            lookup_include = '%s__lt' % self.lookup_field
-            lookup_exclude_pk = 'pk__gte'
-        lookup_exclude = None
-        if pk is not _NO_PK:
-            lookup_include = "%se" % lookup_include
-            lookup_exclude = {self.lookup_field: value, lookup_exclude_pk: pk}
-        lookup_filter = {lookup_include: value}
-        return lookup_filter, lookup_exclude
+            fields.append(
+                ('pk', fields[-1][1]))
+        result = []
+        for f, d in fields:
+            if ((d == DESC and move_to == NEXT_PAGE) or
+                    (d == ASC and move_to == PREV_PAGE)):
+                f = '-%s' % f
+            result.append(f)
+        return result
+
+    # q = X<=? & ~(X=? & ~(Y<?))
+    def _apply_filter(self, i, fields, values, move_to):
+        assert i < len(fields)
+        f, d = fields[i]
+        v = values[i]
+        lf = '%s__gt' % f
+        if ((d == DESC and move_to == NEXT_PAGE) or
+                (d == ASC and move_to == PREV_PAGE)):
+            lf = '%s__lt' % f
+        if len(fields) == 1:
+            return Q(**{lf: v})
+        if i+1 == len(fields):
+            return Q(**{lf: v})
+        q = self._apply_filter(i+1, fields, values, move_to)
+        return Q(**{lf + 'e': v}) & ~(Q(**{f: v}) & ~q)
 
     def apply_filter(self, value, pk, move_to):
-        query_set = self.query_set
-        lookup_filter, lookup_exclude = self.prepare_lookup(
-            value=value, pk=pk, move_to=move_to)
-        query_set = query_set.filter(**lookup_filter)
-        if lookup_exclude:
-            query_set = query_set.exclude(**lookup_exclude)
-        return query_set
+        assert len(value) == len(self.lookup_fields)
+        fields = list(self.fields_direction)
+        values = list(value)
+        if pk is not _NO_PK:
+            values.append(pk)
+            fields.append(
+                ('pk', fields[-1][1]))
+        q = self._apply_filter(0, fields, values, move_to)
+        return self.query_set.filter(q)
 
     def seek(self, value, pk, move_to):
         """
@@ -92,9 +111,36 @@ def seek(self, value, pk, move_to):
             AND NOT (date = ? AND id >= ?)
             ORDER BY date DESC, id DESC
 
+        Multi field lookup. Note how it produces nesting,
+        and how I removed it using boolean logic simplification::
+
+            X <= ?
+            AND NOT (X = ? AND (date <= ? AND NOT (date = ? AND id >= ?)))
+            <--->
+            X <= ?
+            AND (NOT X = ? OR NOT date <= ? OR (date = ? AND id >= ?))
+            <--->
+            X <= ?
+            AND (NOT X = ? OR NOT date <= ? OR date = ?)
+            AND (NOT X = ? OR NOT date <= ? OR id >= ?)
+
+            A * ~(B * (C * ~(D * F)))
+            -> (D + ~B + ~C) * (F + ~B + ~C) * A
+            A * ~(B * (C * ~(D * (F * ~(G * H)))))
+            -> (D + ~B + ~C) * (F + ~B + ~C) * (~B + ~C + ~G + ~H) * A
+            A * ~(B * (C * ~(D * (F * ~(G * (X * ~(Y * Z)))))))
+            -> (D + ~B + ~C) * (F + ~B + ~C) * (Y + ~B + ~C + ~G + ~X) * (Z + ~B + ~C + ~G + ~X) * A
+
+        Addendum::
+
+            X <= ?
+            AND NOT (X = ? AND NOT (date <= ? AND NOT (date = ? AND id >= ?)))
+
         """
         query_set = self.query_set
         if value is not None:
+            if not isinstance(value, (tuple, list)):
+                value = (value,)
             query_set = self.apply_filter(
                 value=value, pk=pk, move_to=move_to)
         query_set = query_set.order_by(
@@ -125,7 +171,6 @@ def page(self, value, pk=_NO_PK, move_to=NEXT_PAGE):
 
 
 class SeekPage(Sequence):
-
     def __init__(self, query_set, key, move_to, paginator):
         self._query_set = query_set
         self._key = key
@@ -163,8 +208,11 @@ def _some_seek(self, direction):
         pk = _NO_PK
         if self._key['pk'] is not _NO_PK:
             pk = last.pk
+        values = tuple(
+            getattr(last, f)
+            for f in self.paginator.fields)
         return self.paginator.seek(
-            value=getattr(last, self.paginator.lookup_field),
+            value=values,
             pk=pk,
             move_to=direction)
 
@@ -214,10 +262,10 @@ def prev_pages_left(self, limit=None):
     def _some_page(self, index):
         if not self.object_list:
             return {}
-        key = {
-            'value': getattr(
-                self.object_list[index],
-                self.paginator.lookup_field)}
+        values = tuple(
+            getattr(self.object_list[index], f)
+            for f in self.paginator.fields)
+        key = {'value': values}
         if self._key['pk'] is not _NO_PK:
             key['pk'] = self.object_list[index].pk
         return key

infinite_scroll_pagination/serializers.py

@@ -63,6 +63,8 @@ def to_page_key(value=None, pk=None):
     """Serialize a value and pk to `timestamp-pk`` format"""
     if value is None:
         return ''
+    if isinstance(value, (tuple, list)):
+        (value,) = value
     value = _make_aware_maybe(value)
     try:
         timestamp = value.timestamp()