From 074ac06dac27ad285ecd9738acb75e5e920fdfd9 Mon Sep 17 00:00:00 2001 From: Pyry Takala <7336413+pyrytakala@users.noreply.github.com> Date: Wed, 26 Nov 2025 23:11:00 +0000 Subject: [PATCH] Fix KeyError when sorting with _labels=on and excluded sort column Handle both IndexError and KeyError when accessing sort column in _next_value_and_url. CustomRow objects (used with _labels=on) raise KeyError for missing columns, while sqlite3.Row raises IndexError. Adds tests for sorting with _labels=on when the sort column is excluded from SELECT via _col or _nocol parameters. --- datasette/views/table.py | 2 +- tests/test_table_api.py | 65 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/datasette/views/table.py b/datasette/views/table.py index 007c0c85..96d60d78 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -1784,7 +1784,7 @@ async def _next_value_and_url( if (sort or sort_desc) and not is_view: try: prefix = rows[-2][sort or sort_desc] - except IndexError: + except (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) diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 653679e4..2f749829 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -339,6 +339,71 @@ async def test_sortable(ds_client, query_string, sort_key, human_description_en) assert [r["content"] for r in expected] == [r["content"] for r in fetched] +async def _assert_pagination_works(ds_client, response): + """Helper to verify pagination works correctly.""" + assert response.status_code == 200 + data = response.json() + assert "rows" in data + assert len(data["rows"]) > 0 + if "next_url" in data and data["next_url"]: + next_path = data["next_url"].replace("http://localhost", "") + next_response = await ds_client.get(next_path) + assert next_response.status_code == 200 + next_data = next_response.json() + assert "rows" in next_data + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "sort_param,exclude_param", + [ + ("_sort=sortable", "_col=content"), + ("_sort=sortable", "_nocol=text&_nocol=sortable_with_nulls"), + ("_sort_desc=sortable", "_col=content"), + ], +) +async def test_labels_sort_excluded_column(ds_client, sort_param, exclude_param): + """Test that sorting works when _labels=on and sort column is excluded from SELECT.""" + path = ( + f"/fixtures/sortable.json" + f"?_labels=on" + f"&{sort_param}" + f"&{exclude_param}" + f"&_shape=objects" + f"&_extra=next_url" + ) + response = await ds_client.get(path) + await _assert_pagination_works(ds_client, response) + + +@pytest.mark.asyncio +async def test_labels_foreign_key_sort_excluded_column(ds_client): + """ + Test sorting with _labels=on when sort column is excluded from SELECT. + Uses facetable which has foreign keys and a primary key. + """ + path = ( + "/fixtures/facetable.json" + "?_labels=on" + "&_sort=state" + "&_col=pk" + "&_col=_city_id" + "&_size=2" + ) + response = await ds_client.get(path) + assert response.status_code == 200 + data = response.json() + assert "rows" in data + assert len(data["rows"]) > 0 + + if "next_url" in data and data["next_url"]: + next_path = data["next_url"].replace("http://localhost", "") + next_response = await ds_client.get(next_path) + assert next_response.status_code == 200 + next_data = next_response.json() + assert "rows" in next_data + + @pytest.mark.asyncio async def test_sortable_and_filtered(ds_client): path = (