simonw/datasette

Pagination crashes when sorting by excluded column with _labels=on (KeyError not caught)

Github

Back to all

Summary

Context: The _next_value_and_url function in datasette/views/table.py handles pagination for table views, including computing the “next” page URL when sorting by columns.
Bug: The function catches IndexError when trying to access a sort column that’s missing from the SELECT, but CustomRow objects raise KeyError instead.
Actual vs. expected: When labels are expanded (?_labels=on), rows become CustomRow objects which raise KeyErrorfor missing keys, but the code only catches IndexError, causing the KeyError to propagate and crash the request.
Impact: Any table view using ?_labels=on with ?_sort=COLUMN where the sort column is excluded via ?_col= results in a 500 Internal Server Error, completely breaking pagination and even the initial page load.

Code with bug

# datasette/views/table.py, lines 1784–1805
if (sort or sort_desc) and not is_view:
    try:
        prefix = rows[-2][sort or sort_desc]
        # <-- BUG 🔴 CustomRow raises KeyError, not IndexError
    except IndexError:
        # <-- BUG 🔴 Should catch (IndexError, KeyError)
        # sort/sort_desc column missing from SELECT - look up value by PK instead
        prefix_where_clause = " and ".join(
            "[{}] = :pk{}".format(pk, i)
            for i, pk in enumerate(pks)
        )

        prefix_lookup_sql = "select [{}] from [{}] where {}".format(
            sort or sort_desc,
            table_name,
            prefix_where_clause,
        )

        prefix = (
            await db.execute(
                prefix_lookup_sql,
                {
                    **{
                        "pk{}".format(i): rows[-2][pk]
                        for i, pk in enumerate(pks)
                    }
                },
            )
        ).single_value()

Failing test

"""Failing test that demonstrates the bug in _next_value_and_url."""
import os
import sqlite3
import tempfile

import pytest
from datasette.app import Datasette


@pytest.mark.asyncio
async def test_pagination_keyerror_bug():
    """
    BUG: Pagination fails with 500 error when using _labels=on with _sort
    on a column not in the SELECT.
    """
    # Create database with foreign key
    db_path = tempfile.mktemp(suffix=".db")
    conn = sqlite3.connect(db_path)

    # Reference table
    conn.execute("CREATE TABLE categories (id INTEGER PRIMARY KEY, name TEXT)")
    conn.execute("INSERT INTO categories VALUES (1, 'A'), (2, 'B')")

    # Main table with FK
    conn.execute(
        """
        CREATE TABLE items (
            pk1 TEXT,
            pk2 TEXT,
            category_id INTEGER,
            content TEXT,
            sortable INTEGER,
            PRIMARY KEY (pk1, pk2),
            FOREIGN KEY (category_id) REFERENCES categories(id)
        )
        """
    )

    # Insert rows for pagination
    for i in range(60):
        pk1 = chr(97 + (i // 26))
        pk2 = chr(97 + (i % 26))
        category = 1 if i % 2 == 0 else 2

        conn.execute(
            "INSERT INTO items VALUES (?, ?, ?, ?, ?)",
            (pk1, pk2, category, f"{pk1}-{pk2}", i),
        )

    conn.commit()
    conn.close()

    try:
        ds = Datasette([db_path])
        db_name = list(ds.databases.keys())[0]

        # Request with:
        # - _labels=on (causes CustomRow)
        # - _sort=sortable (sort by this)
        # - _col=content&_col=category_id (SELECT these, NOT sortable)
        # - _size=10 (ensure pagination)
        response = await ds.client.get(
            f"/{db_name}/items.json"
            "?_labels=on"
            "&_sort=sortable"
            "&_col=content"
            "&_col=category_id"
            "&_size=10"
        )

        # Expected: 200, Actual: 500 (KeyError not caught)
        assert response.status_code == 200, (
            f"BUG: Request failed with {response.status_code}: {response.text}"
        )
    finally:
        if os.path.exists(db_path):
            os.remove(db_path)


if __name__ == "__main__":
    import asyncio

    asyncio.run(test_pagination_keyerror_bug())
    print("Test passed!")

Test output:

test_bug_keyerror.py::test_pagination_keyerror_bug FAILED

    @pytest.mark.asyncio
    async def test_pagination_keyerror_bug():
        ...
>       assert response.status_code == 200, f"First page failed: {response.text}"
E       AssertionError: First page failed: {"ok": false, "error": "'sortable'", "status": 500, "title": null}
E       assert 500 == 200
E        +  where 500 = <Response [500 Internal Server Error]

Recommended fix

Change the exception handling to catch both IndexError and KeyError:

# datasette/views/table.py
if (sort or sort_desc) and not is_view:
    try:
        prefix = rows[-2][sort or sort_desc]
    except (IndexError, KeyError):  # <-- FIX 🟢 Catch both exception types
        # sort/sort_desc column missing from SELECT - look up value by PK instead
        prefix_where_clause = " and ".join(
            "[{}] = :pk{}".format(pk, i)
            for i, pk in enumerate(pks)
        )
        ...