From 4e47a2d894b96854348343374c8e97c9d7055cf6 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 18 Mar 2022 18:37:54 -0700 Subject: [PATCH] Fixed bug where tables with a column called n caused 500 errors Closes #1228 --- datasette/facets.py | 6 +++--- tests/fixtures.py | 33 ++++++++++++++++---------------- tests/test_api.py | 1 + tests/test_csv.py | 32 +++++++++++++++---------------- tests/test_internals_database.py | 10 ++++++++++ tests/test_plugins.py | 6 ++++-- tests/test_table_api.py | 8 ++++++++ 7 files changed, 59 insertions(+), 37 deletions(-) diff --git a/datasette/facets.py b/datasette/facets.py index a1bb4a5f..b15a758c 100644 --- a/datasette/facets.py +++ b/datasette/facets.py @@ -151,10 +151,10 @@ class ColumnFacet(Facet): if column in already_enabled: continue suggested_facet_sql = """ - select {column}, count(*) as n from ( + select {column} as value, count(*) as n from ( {sql} - ) where {column} is not null - group by {column} + ) where value is not null + group by value limit {limit} """.format( column=escape_sqlite(column), sql=self.sql, limit=facet_size + 1 diff --git a/tests/fixtures.py b/tests/fixtures.py index 342a3020..e0e4ec7b 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -564,26 +564,27 @@ CREATE TABLE facetable ( tags text, complex_array text, distinct_some_null, + n text, FOREIGN KEY ("_city_id") REFERENCES [facet_cities](id) ); INSERT INTO facetable - (created, planet_int, on_earth, state, _city_id, _neighborhood, tags, complex_array, distinct_some_null) + (created, planet_int, on_earth, state, _city_id, _neighborhood, tags, complex_array, distinct_some_null, n) VALUES - ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Mission', '["tag1", "tag2"]', '[{"foo": "bar"}]', 'one'), - ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Dogpatch', '["tag1", "tag3"]', '[]', 'two'), - ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'SOMA', '[]', '[]', null), - ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Tenderloin', '[]', '[]', null), - ("2019-01-15 08:00:00", 1, 1, 'CA', 1, 'Bernal Heights', '[]', '[]', null), - ("2019-01-15 08:00:00", 1, 1, 'CA', 1, 'Hayes Valley', '[]', '[]', null), - ("2019-01-15 08:00:00", 1, 1, 'CA', 2, 'Hollywood', '[]', '[]', null), - ("2019-01-15 08:00:00", 1, 1, 'CA', 2, 'Downtown', '[]', '[]', null), - ("2019-01-16 08:00:00", 1, 1, 'CA', 2, 'Los Feliz', '[]', '[]', null), - ("2019-01-16 08:00:00", 1, 1, 'CA', 2, 'Koreatown', '[]', '[]', null), - ("2019-01-16 08:00:00", 1, 1, 'MI', 3, 'Downtown', '[]', '[]', null), - ("2019-01-17 08:00:00", 1, 1, 'MI', 3, 'Greektown', '[]', '[]', null), - ("2019-01-17 08:00:00", 1, 1, 'MI', 3, 'Corktown', '[]', '[]', null), - ("2019-01-17 08:00:00", 1, 1, 'MI', 3, 'Mexicantown', '[]', '[]', null), - ("2019-01-17 08:00:00", 2, 0, 'MC', 4, 'Arcadia Planitia', '[]', '[]', null) + ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Mission', '["tag1", "tag2"]', '[{"foo": "bar"}]', 'one', 'n1'), + ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Dogpatch', '["tag1", "tag3"]', '[]', 'two', 'n2'), + ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'SOMA', '[]', '[]', null, null), + ("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Tenderloin', '[]', '[]', null, null), + ("2019-01-15 08:00:00", 1, 1, 'CA', 1, 'Bernal Heights', '[]', '[]', null, null), + ("2019-01-15 08:00:00", 1, 1, 'CA', 1, 'Hayes Valley', '[]', '[]', null, null), + ("2019-01-15 08:00:00", 1, 1, 'CA', 2, 'Hollywood', '[]', '[]', null, null), + ("2019-01-15 08:00:00", 1, 1, 'CA', 2, 'Downtown', '[]', '[]', null, null), + ("2019-01-16 08:00:00", 1, 1, 'CA', 2, 'Los Feliz', '[]', '[]', null, null), + ("2019-01-16 08:00:00", 1, 1, 'CA', 2, 'Koreatown', '[]', '[]', null, null), + ("2019-01-16 08:00:00", 1, 1, 'MI', 3, 'Downtown', '[]', '[]', null, null), + ("2019-01-17 08:00:00", 1, 1, 'MI', 3, 'Greektown', '[]', '[]', null, null), + ("2019-01-17 08:00:00", 1, 1, 'MI', 3, 'Corktown', '[]', '[]', null, null), + ("2019-01-17 08:00:00", 1, 1, 'MI', 3, 'Mexicantown', '[]', '[]', null, null), + ("2019-01-17 08:00:00", 2, 0, 'MC', 4, 'Arcadia Planitia', '[]', '[]', null, null) ; CREATE TABLE binary_data ( diff --git a/tests/test_api.py b/tests/test_api.py index d3c94023..421bb1fe 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -210,6 +210,7 @@ def test_database_page(app_client): "tags", "complex_array", "distinct_some_null", + "n", ], "primary_keys": ["pk"], "count": 15, diff --git a/tests/test_csv.py b/tests/test_csv.py index 8749cd8b..7fc25a09 100644 --- a/tests/test_csv.py +++ b/tests/test_csv.py @@ -24,22 +24,22 @@ world ) EXPECTED_TABLE_WITH_LABELS_CSV = """ -pk,created,planet_int,on_earth,state,_city_id,_city_id_label,_neighborhood,tags,complex_array,distinct_some_null -1,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Mission,"[""tag1"", ""tag2""]","[{""foo"": ""bar""}]",one -2,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Dogpatch,"[""tag1"", ""tag3""]",[],two -3,2019-01-14 08:00:00,1,1,CA,1,San Francisco,SOMA,[],[], -4,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Tenderloin,[],[], -5,2019-01-15 08:00:00,1,1,CA,1,San Francisco,Bernal Heights,[],[], -6,2019-01-15 08:00:00,1,1,CA,1,San Francisco,Hayes Valley,[],[], -7,2019-01-15 08:00:00,1,1,CA,2,Los Angeles,Hollywood,[],[], -8,2019-01-15 08:00:00,1,1,CA,2,Los Angeles,Downtown,[],[], -9,2019-01-16 08:00:00,1,1,CA,2,Los Angeles,Los Feliz,[],[], -10,2019-01-16 08:00:00,1,1,CA,2,Los Angeles,Koreatown,[],[], -11,2019-01-16 08:00:00,1,1,MI,3,Detroit,Downtown,[],[], -12,2019-01-17 08:00:00,1,1,MI,3,Detroit,Greektown,[],[], -13,2019-01-17 08:00:00,1,1,MI,3,Detroit,Corktown,[],[], -14,2019-01-17 08:00:00,1,1,MI,3,Detroit,Mexicantown,[],[], -15,2019-01-17 08:00:00,2,0,MC,4,Memnonia,Arcadia Planitia,[],[], +pk,created,planet_int,on_earth,state,_city_id,_city_id_label,_neighborhood,tags,complex_array,distinct_some_null,n +1,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Mission,"[""tag1"", ""tag2""]","[{""foo"": ""bar""}]",one,n1 +2,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Dogpatch,"[""tag1"", ""tag3""]",[],two,n2 +3,2019-01-14 08:00:00,1,1,CA,1,San Francisco,SOMA,[],[],, +4,2019-01-14 08:00:00,1,1,CA,1,San Francisco,Tenderloin,[],[],, +5,2019-01-15 08:00:00,1,1,CA,1,San Francisco,Bernal Heights,[],[],, +6,2019-01-15 08:00:00,1,1,CA,1,San Francisco,Hayes Valley,[],[],, +7,2019-01-15 08:00:00,1,1,CA,2,Los Angeles,Hollywood,[],[],, +8,2019-01-15 08:00:00,1,1,CA,2,Los Angeles,Downtown,[],[],, +9,2019-01-16 08:00:00,1,1,CA,2,Los Angeles,Los Feliz,[],[],, +10,2019-01-16 08:00:00,1,1,CA,2,Los Angeles,Koreatown,[],[],, +11,2019-01-16 08:00:00,1,1,MI,3,Detroit,Downtown,[],[],, +12,2019-01-17 08:00:00,1,1,MI,3,Detroit,Greektown,[],[],, +13,2019-01-17 08:00:00,1,1,MI,3,Detroit,Corktown,[],[],, +14,2019-01-17 08:00:00,1,1,MI,3,Detroit,Mexicantown,[],[],, +15,2019-01-17 08:00:00,2,0,MC,4,Memnonia,Arcadia Planitia,[],[],, """.lstrip().replace( "\n", "\r\n" ) diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index 31538a24..551f67e1 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -86,6 +86,7 @@ async def test_table_exists(db, tables, exists): "tags", "complex_array", "distinct_some_null", + "n", ], ), ( @@ -204,6 +205,15 @@ async def test_table_columns(db, table, expected): is_pk=0, hidden=0, ), + Column( + cid=10, + name="n", + type="text", + notnull=0, + default_value=None, + is_pk=0, + hidden=0, + ), ], ), ( diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 656f39e4..15bde962 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -442,6 +442,7 @@ def test_hook_register_output_renderer_all_parameters(app_client): "tags", "complex_array", "distinct_some_null", + "n", ], "rows": [ "", @@ -460,7 +461,7 @@ def test_hook_register_output_renderer_all_parameters(app_client): "", "", ], - "sql": "select pk, created, planet_int, on_earth, state, _city_id, _neighborhood, tags, complex_array, distinct_some_null from facetable order by pk limit 51", + "sql": "select pk, created, planet_int, on_earth, state, _city_id, _neighborhood, tags, complex_array, distinct_some_null, n from facetable order by pk limit 51", "query_name": None, "database": "fixtures", "table": "facetable", @@ -531,8 +532,9 @@ def test_hook_register_output_renderer_can_render(app_client): "tags", "complex_array", "distinct_some_null", + "n", ], - "sql": "select pk, created, planet_int, on_earth, state, _city_id, _neighborhood, tags, complex_array, distinct_some_null from facetable order by pk limit 51", + "sql": "select pk, created, planet_int, on_earth, state, _city_id, _neighborhood, tags, complex_array, distinct_some_null, n from facetable order by pk limit 51", "query_name": None, "database": "fixtures", "table": "facetable", diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 3d0a7fbd..9db383c3 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -532,6 +532,7 @@ def test_table_filter_json_arraycontains(app_client): '["tag1", "tag2"]', '[{"foo": "bar"}]', "one", + "n1", ], [ 2, @@ -544,6 +545,7 @@ def test_table_filter_json_arraycontains(app_client): '["tag1", "tag3"]', "[]", "two", + "n2", ], ] @@ -565,6 +567,7 @@ def test_table_filter_json_arraynotcontains(app_client): '["tag1", "tag2"]', '[{"foo": "bar"}]', "one", + "n1", ] ] @@ -585,6 +588,7 @@ def test_table_filter_extra_where(app_client): '["tag1", "tag3"]', "[]", "two", + "n2", ] ] == response.json["rows"] @@ -958,6 +962,7 @@ def test_expand_labels(app_client): "tags": '["tag1", "tag3"]', "complex_array": "[]", "distinct_some_null": "two", + "n": "n2", }, "13": { "pk": 13, @@ -970,6 +975,7 @@ def test_expand_labels(app_client): "tags": "[]", "complex_array": "[]", "distinct_some_null": None, + "n": None, }, } == response.json @@ -1161,6 +1167,7 @@ def test_generated_columns_are_visible_in_datasette(): "tags", "complex_array", "distinct_some_null", + "n", ], ), ( @@ -1188,6 +1195,7 @@ def test_generated_columns_are_visible_in_datasette(): "tags", "complex_array", "distinct_some_null", + "n", ], ), (