From 0ac8f608601e16fc9f8be772aa4e57b250a283d5 Mon Sep 17 00:00:00 2001 From: feiiiiii5 Date: Wed, 30 Sep 2026 21:18:27 +0800 Subject: [PATCH] Fall back to rowid when paginating a sort prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _next_value_and_url() re-reads the sort column by primary key when that column is not in the SELECT. For a table with no primary key `pks` is `[]`, so the WHERE clause came out empty and the lookup SQL was `select b from items where ` — sqlite3.OperationalError: incomplete input, and a 500. Such tables paginate on rowid, which path_from_row_pks() and the use_rowid handling above already use. Use the same key in the fallback, so the next value is identical to the one the non-fallback path produces. Fixes #2974 --- datasette/views/table.py | 6 +++-- tests/test_table_api.py | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/datasette/views/table.py b/datasette/views/table.py index 0a908c69..d2bf173f 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -2518,8 +2518,10 @@ async def _next_value_and_url( prefix = rows[-2][sort or sort_desc] except IndexError: # sort/sort_desc column missing from SELECT - look up value by PK instead + # Tables with no primary key paginate on rowid + lookup_pks = pks or ["rowid"] prefix_where_clause = " and ".join( - f"{escape_sqlite(pk)} = :pk{i}" for i, pk in enumerate(pks) + f"{escape_sqlite(pk)} = :pk{i}" for i, pk in enumerate(lookup_pks) ) prefix_lookup_sql = ( f"select {escape_sqlite(sort or sort_desc)} " @@ -2528,7 +2530,7 @@ async def _next_value_and_url( prefix = ( await db.execute( prefix_lookup_sql, - {**{f"pk{i}": rows[-2][pk] for i, pk in enumerate(pks)}}, + {**{f"pk{i}": rows[-2][pk] for i, pk in enumerate(lookup_pks)}}, ) ).single_value() if isinstance(prefix, dict) and "value" in prefix: diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 58a5ddc3..d5f525b2 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -3,6 +3,7 @@ import urllib import pytest +from datasette.app import Datasette from datasette.fixtures import generate_compound_rows, generate_sortable_rows from datasette.utils import detect_json1, tilde_encode from datasette.utils.sqlite import sqlite_version @@ -1615,6 +1616,53 @@ async def test_col_nocol_errors(ds_client, path, expected_error): assert response.json()["error"] == expected_error +@pytest.mark.asyncio +@pytest.mark.parametrize( + "query_string,expected_order", + ( + ("_sort=b&_nocol=b", list(range(1, 26))), + ("_sort=b&_col=c", list(range(1, 26))), + ("_sort_desc=b&_nocol=b", list(range(25, 0, -1))), + ), +) +async def test_sort_excluded_column_rowid_table(query_string, expected_order): + # Tables with no explicit primary key paginate on rowid, so the + # "look the sort value up by primary key" fallback - which exists so that + # sorting works when the sort column is not in the SELECT, added for #1773 - + # has to fall back to rowid too. + # https://github.com/simonw/datasette/issues/2974 + ds = Datasette(settings={"default_page_size": 10, "max_returned_rows": 1000}) + db = ds.add_memory_database("rowid_pagination") + # c mirrors b and is never excluded, so it can be used to check the + # order the rows were paginated in. Values are zero-padded so that + # string order matches numeric order. + await db.execute_write("create table items (a text, b text, c text)") + for i in range(1, 26): + await db.execute_write( + "insert into items (a, b, c) values (?, ?, ?)", + (f"a{i:02d}", f"b{i:02d}", f"c{i:02d}"), + ) + await ds.invoke_startup() + + fetched = [] + path = f"/rowid_pagination/items.json?_shape=objects&{query_string}" + for _ in range(100): + response = await ds.client.get(path) + assert response.status_code == 200, response.text + data = response.json() + fetched.extend(data["rows"]) + path = data["next_url"] + if not path: + break + path = path.replace("http://localhost", "") + else: + assert False, "pagination did not terminate" + + # Every row visited exactly once, in the requested order, with no rows + # skipped or repeated at a page boundary + assert [row["c"] for row in fetched] == [f"c{i:02d}" for i in expected_order] + + @pytest.mark.asyncio @pytest.mark.parametrize( "extra,expected_json",