mirror of
https://github.com/simonw/datasette.git
synced 2026-10-18 15:26:57 +02:00
Fall back to rowid when paginating a sort prefix
_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
This commit is contained in:
parent
cec5e6b2ef
commit
0ac8f60860
2 changed files with 52 additions and 2 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue