Fixed bug where tables with a column called n caused 500 errors

Closes #1228
This commit is contained in:
Simon Willison 2022-03-18 18:37:54 -07:00
commit 4e47a2d894
7 changed files with 59 additions and 37 deletions

View file

@ -151,10 +151,10 @@ class ColumnFacet(Facet):
if column in already_enabled: if column in already_enabled:
continue continue
suggested_facet_sql = """ suggested_facet_sql = """
select {column}, count(*) as n from ( select {column} as value, count(*) as n from (
{sql} {sql}
) where {column} is not null ) where value is not null
group by {column} group by value
limit {limit} limit {limit}
""".format( """.format(
column=escape_sqlite(column), sql=self.sql, limit=facet_size + 1 column=escape_sqlite(column), sql=self.sql, limit=facet_size + 1

View file

@ -564,26 +564,27 @@ CREATE TABLE facetable (
tags text, tags text,
complex_array text, complex_array text,
distinct_some_null, distinct_some_null,
n text,
FOREIGN KEY ("_city_id") REFERENCES [facet_cities](id) FOREIGN KEY ("_city_id") REFERENCES [facet_cities](id)
); );
INSERT INTO facetable 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 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, 'Mission', '["tag1", "tag2"]', '[{"foo": "bar"}]', 'one', 'n1'),
("2019-01-14 08:00:00", 1, 1, 'CA', 1, 'Dogpatch', '["tag1", "tag3"]', '[]', 'two'), ("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), ("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), ("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), ("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), ("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), ("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), ("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), ("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), ("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), ("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), ("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), ("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), ("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) ("2019-01-17 08:00:00", 2, 0, 'MC', 4, 'Arcadia Planitia', '[]', '[]', null, null)
; ;
CREATE TABLE binary_data ( CREATE TABLE binary_data (

View file

@ -210,6 +210,7 @@ def test_database_page(app_client):
"tags", "tags",
"complex_array", "complex_array",
"distinct_some_null", "distinct_some_null",
"n",
], ],
"primary_keys": ["pk"], "primary_keys": ["pk"],
"count": 15, "count": 15,

View file

@ -24,22 +24,22 @@ world
) )
EXPECTED_TABLE_WITH_LABELS_CSV = """ EXPECTED_TABLE_WITH_LABELS_CSV = """
pk,created,planet_int,on_earth,state,_city_id,_city_id_label,_neighborhood,tags,complex_array,distinct_some_null 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 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 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 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,[],[], 15,2019-01-17 08:00:00,2,0,MC,4,Memnonia,Arcadia Planitia,[],[],,
""".lstrip().replace( """.lstrip().replace(
"\n", "\r\n" "\n", "\r\n"
) )

View file

@ -86,6 +86,7 @@ async def test_table_exists(db, tables, exists):
"tags", "tags",
"complex_array", "complex_array",
"distinct_some_null", "distinct_some_null",
"n",
], ],
), ),
( (
@ -204,6 +205,15 @@ async def test_table_columns(db, table, expected):
is_pk=0, is_pk=0,
hidden=0, hidden=0,
), ),
Column(
cid=10,
name="n",
type="text",
notnull=0,
default_value=None,
is_pk=0,
hidden=0,
),
], ],
), ),
( (

View file

@ -442,6 +442,7 @@ def test_hook_register_output_renderer_all_parameters(app_client):
"tags", "tags",
"complex_array", "complex_array",
"distinct_some_null", "distinct_some_null",
"n",
], ],
"rows": [ "rows": [
"<sqlite3.Row object at 0xXXX>", "<sqlite3.Row object at 0xXXX>",
@ -460,7 +461,7 @@ def test_hook_register_output_renderer_all_parameters(app_client):
"<sqlite3.Row object at 0xXXX>", "<sqlite3.Row object at 0xXXX>",
"<sqlite3.Row object at 0xXXX>", "<sqlite3.Row object at 0xXXX>",
], ],
"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, "query_name": None,
"database": "fixtures", "database": "fixtures",
"table": "facetable", "table": "facetable",
@ -531,8 +532,9 @@ def test_hook_register_output_renderer_can_render(app_client):
"tags", "tags",
"complex_array", "complex_array",
"distinct_some_null", "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, "query_name": None,
"database": "fixtures", "database": "fixtures",
"table": "facetable", "table": "facetable",

View file

@ -532,6 +532,7 @@ def test_table_filter_json_arraycontains(app_client):
'["tag1", "tag2"]', '["tag1", "tag2"]',
'[{"foo": "bar"}]', '[{"foo": "bar"}]',
"one", "one",
"n1",
], ],
[ [
2, 2,
@ -544,6 +545,7 @@ def test_table_filter_json_arraycontains(app_client):
'["tag1", "tag3"]', '["tag1", "tag3"]',
"[]", "[]",
"two", "two",
"n2",
], ],
] ]
@ -565,6 +567,7 @@ def test_table_filter_json_arraynotcontains(app_client):
'["tag1", "tag2"]', '["tag1", "tag2"]',
'[{"foo": "bar"}]', '[{"foo": "bar"}]',
"one", "one",
"n1",
] ]
] ]
@ -585,6 +588,7 @@ def test_table_filter_extra_where(app_client):
'["tag1", "tag3"]', '["tag1", "tag3"]',
"[]", "[]",
"two", "two",
"n2",
] ]
] == response.json["rows"] ] == response.json["rows"]
@ -958,6 +962,7 @@ def test_expand_labels(app_client):
"tags": '["tag1", "tag3"]', "tags": '["tag1", "tag3"]',
"complex_array": "[]", "complex_array": "[]",
"distinct_some_null": "two", "distinct_some_null": "two",
"n": "n2",
}, },
"13": { "13": {
"pk": 13, "pk": 13,
@ -970,6 +975,7 @@ def test_expand_labels(app_client):
"tags": "[]", "tags": "[]",
"complex_array": "[]", "complex_array": "[]",
"distinct_some_null": None, "distinct_some_null": None,
"n": None,
}, },
} == response.json } == response.json
@ -1161,6 +1167,7 @@ def test_generated_columns_are_visible_in_datasette():
"tags", "tags",
"complex_array", "complex_array",
"distinct_some_null", "distinct_some_null",
"n",
], ],
), ),
( (
@ -1188,6 +1195,7 @@ def test_generated_columns_are_visible_in_datasette():
"tags", "tags",
"complex_array", "complex_array",
"distinct_some_null", "distinct_some_null",
"n",
], ],
), ),
( (