Skip to content

Warn about importance of ordering for all pagination classes - #8494

Open
Elkasitu wants to merge 7 commits into
encode:mainfrom
Elkasitu:improve-pagination-documentation
Open

Warn about importance of ordering for all pagination classes#8494
Elkasitu wants to merge 7 commits into
encode:mainfrom
Elkasitu:improve-pagination-documentation

Conversation

@Elkasitu

Copy link
Copy Markdown

The ordering recommendations given for the CursorPagination scheme
actually apply to all pagination schemes, an unsuspecting developer that
implements the more common LimitOffsetPagination or
PageNumberPagination classes is unlikely to be aware of the importance
of consistent ordering.

This commit moves the Details and limitations section out of the
CursorPagination section and puts it as the very first subsection of
the Pagination page so that it's one of the first things that
developers see.

Some examples of inconsistencies as well as how to deal with them are
given, and an extra way to change the ordering of a paginated view is
provided.

Fixes #6886

Hopefully this will prevent future developers from scratching their head and wondering why despite a view returning a count of X, going through all the pages yields Y records.

The ordering recommendations given for the CursorPagination scheme
actually apply to all pagination schemes, an unsuspecting developer that
implements the more common `LimitOffsetPagination` or
`PageNumberPagination` classes is unlikely to be aware of the importance
of consistent ordering.

This commit moves the `Details and limitations` section out of the
`CursorPagination` section and puts it as the very first subsection of
the `Pagination` page so that it's one of the first things that
developers see.

Some examples of inconsistencies as well as how to deal with them are
given, and an extra way to change the ordering of a paginated view is
provided.

Fixes encode#6886
@lovelydinosaur

Copy link
Copy Markdown
Contributor

I'm not convinced that this section makes sense when moved around like this.

For example from a first look over - it discusses the ordering attribute on the pagination class. Which is specific to the CursorPagination class, rather than being generally applicable.

@Elkasitu

Elkasitu commented Jun 6, 2022

Copy link
Copy Markdown
Author

You're right, when I wrote this patch I hadn't actually tried to implement CursorPagination yet so I assumed some of these things also applied to the other Pagination classes (like an interface).

I have made some changes, I kept it as a general section because the only Pagination class that is really different from the others is CursorPagination, so having a subsection per class seemed like a lot of duplicated text.

@Faki-140386

Copy link
Copy Markdown

Segue sempre sem fazer nenhuma alteração

@stale

stale Bot commented Aug 11, 2022

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Aug 11, 2022
@Elkasitu

Elkasitu commented Aug 14, 2022

Copy link
Copy Markdown
Author

@tomchristie any chance of ever getting something like this merged? I feel like it's an easy and dangerous trap to fall into

@stale stale Bot removed the stale label Aug 14, 2022
@auvipy

auvipy commented Nov 22, 2022

Copy link
Copy Markdown
Collaborator

@tomchristie any chance of ever getting something like this merged? I feel like it's an easy and dangerous trap to fall into

did you addressed the concerns @tomchristie initially raised?

@auvipy
auvipy self-requested a review November 27, 2022 12:57
@Elkasitu

Copy link
Copy Markdown
Author

@auvipy Well there was only one concern which was about the ordering attribute of the CursorPagination class which I did address, you can see the changes I made in the second commit (can be squashed).

If there are any other concerns I'm willing to address them too

auvipy
auvipy previously requested changes Dec 1, 2022
Comment thread docs/api-guide/pagination.md Outdated

If the main field that you wish to order by does not satisfy these conditions, you can order by multiple fields, as long as one of the fields fulfills all of the conditions above the result set should remain consistent across database calls.

# inconsistent

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to include for elaborate code example here including the pagination classes

@stale

stale Bot commented May 6, 2025

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label May 6, 2025
@ulgens

ulgens commented May 6, 2025

Copy link
Copy Markdown
Contributor

Still looks relevant.

@stale stale Bot removed the stale label May 6, 2025
@stale

stale Bot commented Oct 18, 2025

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the stale label Oct 18, 2025
@Natgho

Natgho commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

I cannot edit the PR, but the following examples can be added;

the lack of a unique, unchanging ordering field can cause records to shift between pages during navigation, especially if the name or status fields change or have duplicate values.

from django.db import models
from rest_framework import pagination, serializers, viewsets

# --- INCONSISTENT EXAMPLES ---

class Product(models.Model):
    name = models.CharField(max_length=100)
    category = models.CharField(max_length=100)
    # No unique/indexed field used for ordering

class ProductSerializer(serializers.ModelSerializer):
    class Meta:
        model = Product
        fields = '__all__'

# 1. Inconsistent LimitOffsetPagination
class UnreliableProductViewSet(viewsets.ModelViewSet):
    queryset = Product.objects.all().order_by('category') # Many duplicates, inconsistent pages
    serializer_class = ProductSerializer
    pagination_class = pagination.LimitOffsetPagination

# 2. Inconsistent CursorPagination (Will fail or be buggy)
class RiskyCursorPagination(pagination.CursorPagination):
    page_size = 10
    ordering = 'name'  # Problematic: 'name' is not unique and can change

By ensuring the final ordering field is unique and indexed (like id or a created timestamp), we guarantee that the cursor position remains stable:

# --- CONSISTENT EXAMPLES ---

class SecureProduct(models.Model):
    name = models.CharField(max_length=100)
    created_at = models.DateTimeField(auto_now_add=True, db_index=True)

class SecureProductSerializer(serializers.ModelSerializer):
    class Meta:
        model = SecureProduct
        fields = '__all__'

# 1. Consistent PageNumberPagination with Multiple Ordering
class ReliableProductViewSet(viewsets.ModelViewSet):
    # Combining a non-unique field with a unique ID ensures consistency
    queryset = SecureProduct.objects.all().order_by('name', 'id')
    serializer_class = SecureProductSerializer
    pagination_class = pagination.PageNumberPagination

# 2. Recommended CursorPagination Usage
class StandardCursorPagination(pagination.CursorPagination):
    page_size = 10
    ordering = '-created_at' # Unique-ish, indexed, and unchanging

class AccurateProductViewSet(viewsets.ModelViewSet):
    queryset = SecureProduct.objects.all()
    serializer_class = SecureProductSerializer
    pagination_class = StandardCursorPagination

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing/duplicate records when using ordering and LimitOffsetPagination

8 participants