simonw/datasette
Pagination crashes when sorting by excluded column with _labels=on (KeyError not caught)
Summary
Context: The
_next_value_and_urlfunction indatasette/views/table.pyhandles pagination for table views, including computing the “next” page URL when sorting by columns.Bug: The function catches
IndexErrorwhen trying to access a sort column that’s missing from the SELECT, butCustomRowobjects raiseKeyErrorinstead.Actual vs. expected: When labels are expanded (
?_labels=on), rows becomeCustomRowobjects which raiseKeyErrorfor missing keys, but the code only catchesIndexError, causing theKeyErrorto propagate and crash the request.Impact: Any table view using
?_labels=onwith?_sort=COLUMNwhere 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()
# 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()
# 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!")
"""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!")
"""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]
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]
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) ) ...
# 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) ) ...
# 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) ) ...