Correct facet links for columns with a leading underscore, closes #1506

datasette/facets.py

@@ -237,14 +237,17 @@ class ColumnFacet(Facet):
                 else:
                     expanded = {}
                 for row in facet_rows:
-                    selected = (column, str(row["value"])) in qs_pairs
+                    column_qs = column
+                    if column.startswith("_"):
+                        column_qs = "{}__exact".format(column)
+                    selected = (column_qs, str(row["value"])) in qs_pairs
                     if selected:
                         toggle_path = path_with_removed_args(
-                            self.request, {column: str(row["value"])}
+                            self.request, {column_qs: str(row["value"])}
                         )
                     else:
                         toggle_path = path_with_added_args(
-                            self.request, {column: row["value"]}
+                            self.request, {column_qs: row["value"]}
                         )
                     facet_results_values.append(
                         {

tests/fixtures.py

@@ -355,12 +355,12 @@ METADATA = {
                 "neighborhood_search": {
                     "sql": textwrap.dedent(
                         """
-                        select neighborhood, facet_cities.name, state
+                        select _neighborhood, facet_cities.name, state
                         from facetable
                             join facet_cities
                                 on facetable.city_id = facet_cities.id
-                        where neighborhood like '%' || :text || '%'
+                        where _neighborhood like '%' || :text || '%'
-                        order by neighborhood;
+                        order by _neighborhood;
                     """
                     ),
                     "title": "Search neighborhoods",
@@ -559,14 +559,14 @@ CREATE TABLE facetable (
     on_earth integer,
     state text,
     city_id integer,
-    neighborhood text,
+    _neighborhood text,
     tags text,
     complex_array text,
     distinct_some_null,
     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)
 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'),

tests/test_api.py

@@ -213,7 +213,7 @@ def test_database_page(app_client):
                 "on_earth",
                 "state",
                 "city_id",
-                "neighborhood",
+                "_neighborhood",
                 "tags",
                 "complex_array",
                 "distinct_some_null",
@@ -1241,7 +1241,9 @@ def test_table_filter_json_arraynotcontains(app_client):
 
 
 def test_table_filter_extra_where(app_client):
-    response = app_client.get("/fixtures/facetable.json?_where=neighborhood='Dogpatch'")
+    response = app_client.get(
+        "/fixtures/facetable.json?_where=_neighborhood='Dogpatch'"
+    )
     assert [
         [
             2,
@@ -1259,14 +1261,16 @@ def test_table_filter_extra_where(app_client):
 
 
 def test_table_filter_extra_where_invalid(app_client):
-    response = app_client.get("/fixtures/facetable.json?_where=neighborhood=Dogpatch'")
+    response = app_client.get("/fixtures/facetable.json?_where=_neighborhood=Dogpatch'")
     assert 400 == response.status
     assert "Invalid SQL" == response.json["title"]
 
 
 def test_table_filter_extra_where_disabled_if_no_sql_allowed():
     with make_app_client(metadata={"allow_sql": {}}) as client:
-        response = client.get("/fixtures/facetable.json?_where=neighborhood='Dogpatch'")
+        response = client.get(
+            "/fixtures/facetable.json?_where=_neighborhood='Dogpatch'"
+        )
         assert 403 == response.status
         assert "_where= is not allowed" == response.json["error"]
 
@@ -1696,7 +1700,7 @@ def test_suggested_facets(app_client):
         {"name": "on_earth", "querystring": "_facet=on_earth"},
         {"name": "state", "querystring": "_facet=state"},
         {"name": "city_id", "querystring": "_facet=city_id"},
-        {"name": "neighborhood", "querystring": "_facet=neighborhood"},
+        {"name": "_neighborhood", "querystring": "_facet=_neighborhood"},
         {"name": "tags", "querystring": "_facet=tags"},
         {"name": "complex_array", "querystring": "_facet=complex_array"},
         {"name": "created", "querystring": "_facet_date=created"},
@@ -1752,7 +1756,7 @@ def test_nocount_nofacet_if_shape_is_object(app_client_with_trace):
 def test_expand_labels(app_client):
     response = app_client.get(
         "/fixtures/facetable.json?_shape=object&_labels=1&_size=2"
-        "&neighborhood__contains=c"
+        "&_neighborhood__contains=c"
     )
     assert {
         "2": {
@@ -1762,7 +1766,7 @@ def test_expand_labels(app_client):
             "on_earth": 1,
             "state": "CA",
             "city_id": {"value": 1, "label": "San Francisco"},
-            "neighborhood": "Dogpatch",
+            "_neighborhood": "Dogpatch",
             "tags": '["tag1", "tag3"]',
             "complex_array": "[]",
             "distinct_some_null": "two",
@@ -1774,7 +1778,7 @@ def test_expand_labels(app_client):
             "on_earth": 1,
             "state": "MI",
             "city_id": {"value": 3, "label": "Detroit"},
-            "neighborhood": "Corktown",
+            "_neighborhood": "Corktown",
             "tags": "[]",
             "complex_array": "[]",
             "distinct_some_null": None,
@@ -2125,7 +2129,7 @@ def test_http_options_request(app_client):
                 "on_earth",
                 "state",
                 "city_id",
-                "neighborhood",
+                "_neighborhood",
                 "tags",
                 "complex_array",
                 "distinct_some_null",
@@ -2152,7 +2156,7 @@ def test_http_options_request(app_client):
                 "planet_int",
                 "on_earth",
                 "city_id",
-                "neighborhood",
+                "_neighborhood",
                 "tags",
                 "complex_array",
                 "distinct_some_null",

tests/test_csv.py

@@ -24,7 +24,7 @@ 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
+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,[],[],

tests/test_facets.py

@@ -23,7 +23,10 @@ async def test_column_facet_suggest(app_client):
         {"name": "on_earth", "toggle_url": "http://localhost/?_facet=on_earth"},
         {"name": "state", "toggle_url": "http://localhost/?_facet=state"},
         {"name": "city_id", "toggle_url": "http://localhost/?_facet=city_id"},
-        {"name": "neighborhood", "toggle_url": "http://localhost/?_facet=neighborhood"},
+        {
+            "name": "_neighborhood",
+            "toggle_url": "http://localhost/?_facet=_neighborhood",
+        },
         {"name": "tags", "toggle_url": "http://localhost/?_facet=tags"},
         {
             "name": "complex_array",
@@ -56,8 +59,8 @@ async def test_column_facet_suggest_skip_if_already_selected(app_client):
             "toggle_url": "http://localhost/?_facet=planet_int&_facet=on_earth&_facet=city_id",
         },
         {
-            "name": "neighborhood",
+            "name": "_neighborhood",
-            "toggle_url": "http://localhost/?_facet=planet_int&_facet=on_earth&_facet=neighborhood",
+            "toggle_url": "http://localhost/?_facet=planet_int&_facet=on_earth&_facet=_neighborhood",
         },
         {
             "name": "tags",
@@ -86,7 +89,7 @@ async def test_column_facet_suggest_skip_if_enabled_by_metadata(app_client):
         "planet_int",
         "on_earth",
         "state",
-        "neighborhood",
+        "_neighborhood",
         "tags",
         "complex_array",
     ] == suggestions
@@ -144,6 +147,128 @@ async def test_column_facet_results(app_client):
     } == buckets
 
 
+@pytest.mark.asyncio
+async def test_column_facet_results_column_starts_with_underscore(app_client):
+    facet = ColumnFacet(
+        app_client.ds,
+        Request.fake("/?_facet=_neighborhood"),
+        database="fixtures",
+        sql="select * from facetable",
+        table="facetable",
+    )
+    buckets, timed_out = await facet.facet_results()
+    assert [] == timed_out
+    assert buckets == {
+        "_neighborhood": {
+            "name": "_neighborhood",
+            "type": "column",
+            "hideable": True,
+            "toggle_url": "/",
+            "results": [
+                {
+                    "value": "Downtown",
+                    "label": "Downtown",
+                    "count": 2,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Downtown",
+                    "selected": False,
+                },
+                {
+                    "value": "Arcadia Planitia",
+                    "label": "Arcadia Planitia",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Arcadia+Planitia",
+                    "selected": False,
+                },
+                {
+                    "value": "Bernal Heights",
+                    "label": "Bernal Heights",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Bernal+Heights",
+                    "selected": False,
+                },
+                {
+                    "value": "Corktown",
+                    "label": "Corktown",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Corktown",
+                    "selected": False,
+                },
+                {
+                    "value": "Dogpatch",
+                    "label": "Dogpatch",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Dogpatch",
+                    "selected": False,
+                },
+                {
+                    "value": "Greektown",
+                    "label": "Greektown",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Greektown",
+                    "selected": False,
+                },
+                {
+                    "value": "Hayes Valley",
+                    "label": "Hayes Valley",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Hayes+Valley",
+                    "selected": False,
+                },
+                {
+                    "value": "Hollywood",
+                    "label": "Hollywood",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Hollywood",
+                    "selected": False,
+                },
+                {
+                    "value": "Koreatown",
+                    "label": "Koreatown",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Koreatown",
+                    "selected": False,
+                },
+                {
+                    "value": "Los Feliz",
+                    "label": "Los Feliz",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Los+Feliz",
+                    "selected": False,
+                },
+                {
+                    "value": "Mexicantown",
+                    "label": "Mexicantown",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Mexicantown",
+                    "selected": False,
+                },
+                {
+                    "value": "Mission",
+                    "label": "Mission",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Mission",
+                    "selected": False,
+                },
+                {
+                    "value": "SOMA",
+                    "label": "SOMA",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=SOMA",
+                    "selected": False,
+                },
+                {
+                    "value": "Tenderloin",
+                    "label": "Tenderloin",
+                    "count": 1,
+                    "toggle_url": "http://localhost/?_facet=_neighborhood&_neighborhood__exact=Tenderloin",
+                    "selected": False,
+                },
+            ],
+            "truncated": False,
+        }
+    }
+
+
 @pytest.mark.asyncio
 async def test_column_facet_from_metadata_cannot_be_hidden(app_client):
     facet = ColumnFacet(

tests/test_html.py

@@ -235,7 +235,10 @@ def test_table_cell_truncation():
             "Corkt…",
             "Mexic…",
             "Arcad…",
-        ] == [td.string for td in table.findAll("td", {"class": "col-neighborhood"})]
+        ] == [
+            td.string
+            for td in table.findAll("td", {"class": "col-neighborhood-b352a7"})
+        ]
 
 
 def test_row_page_does_not_truncate():
@@ -245,7 +248,8 @@ def test_row_page_does_not_truncate():
         table = Soup(response.body, "html.parser").find("table")
         assert table["class"] == ["rows-and-columns"]
         assert ["Mission"] == [
-            td.string for td in table.findAll("td", {"class": "col-neighborhood"})
+            td.string
+            for td in table.findAll("td", {"class": "col-neighborhood-b352a7"})
         ]
 
 
@@ -1312,7 +1316,7 @@ def test_canned_query_show_hide_metadata_option(
 
 def test_extra_where_clauses(app_client):
     response = app_client.get(
-        "/fixtures/facetable?_where=neighborhood='Dogpatch'&_where=city_id=1"
+        "/fixtures/facetable?_where=_neighborhood='Dogpatch'&_where=city_id=1"
     )
     soup = Soup(response.body, "html.parser")
     div = soup.select(".extra-wheres")[0]
@@ -1320,12 +1324,12 @@ def test_extra_where_clauses(app_client):
     hrefs = [a["href"] for a in div.findAll("a")]
     assert [
         "/fixtures/facetable?_where=city_id%3D1",
-        "/fixtures/facetable?_where=neighborhood%3D%27Dogpatch%27",
+        "/fixtures/facetable?_where=_neighborhood%3D%27Dogpatch%27",
     ] == hrefs
     # These should also be persisted as hidden fields
     inputs = soup.find("form").findAll("input")
     hiddens = [i for i in inputs if i["type"] == "hidden"]
-    assert [("_where", "neighborhood='Dogpatch'"), ("_where", "city_id=1")] == [
+    assert [("_where", "_neighborhood='Dogpatch'"), ("_where", "city_id=1")] == [
         (hidden["name"], hidden["value"]) for hidden in hiddens
     ]
 
@@ -1634,11 +1638,11 @@ def test_base_url_affects_metadata_extra_css_urls(app_client_base_url_prefix):
     [
         (
             "/fixtures/neighborhood_search",
-            "/fixtures?sql=%0Aselect+neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+neighborhood%3B%0A&text=",
+            "/fixtures?sql=%0Aselect+_neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+_neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+_neighborhood%3B%0A&text=",
         ),
         (
             "/fixtures/neighborhood_search?text=ber",
-            "/fixtures?sql=%0Aselect+neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+neighborhood%3B%0A&text=ber",
+            "/fixtures?sql=%0Aselect+_neighborhood%2C+facet_cities.name%2C+state%0Afrom+facetable%0A++++join+facet_cities%0A++++++++on+facetable.city_id+%3D+facet_cities.id%0Awhere+_neighborhood+like+%27%25%27+%7C%7C+%3Atext+%7C%7C+%27%25%27%0Aorder+by+_neighborhood%3B%0A&text=ber",
         ),
         ("/fixtures/pragma_cache_size", None),
         (
@@ -1716,23 +1720,23 @@ def test_navigation_menu_links(
         (
             5,
             # Default should show 2 facets
-            "/fixtures/facetable?_facet=neighborhood",
+            "/fixtures/facetable?_facet=_neighborhood",
             2,
             True,
-            "/fixtures/facetable?_facet=neighborhood&_facet_size=max",
+            "/fixtures/facetable?_facet=_neighborhood&_facet_size=max",
         ),
         # _facet_size above max_returned_rows should show max_returned_rows (5)
         (
             5,
-            "/fixtures/facetable?_facet=neighborhood&_facet_size=50",
+            "/fixtures/facetable?_facet=_neighborhood&_facet_size=50",
             5,
             True,
-            "/fixtures/facetable?_facet=neighborhood&_facet_size=max",
+            "/fixtures/facetable?_facet=_neighborhood&_facet_size=max",
         ),
         # If max_returned_rows is high enough, should return all
         (
             20,
-            "/fixtures/facetable?_facet=neighborhood&_facet_size=max",
+            "/fixtures/facetable?_facet=_neighborhood&_facet_size=max",
             14,
             False,
             None,
@@ -1741,7 +1745,7 @@ def test_navigation_menu_links(
         # _facet_size above max_returned_rows should show max_returned_rows (5)
         (
             5,
-            "/fixtures/facetable?_facet=neighborhood&_facet_size=max",
+            "/fixtures/facetable?_facet=_neighborhood&_facet_size=max",
             5,
             True,
             None,
@@ -1760,7 +1764,7 @@ def test_facet_more_links(
     ) as client:
         response = client.get(path)
         soup = Soup(response.body, "html.parser")
-        lis = soup.select("#facet-neighborhood ul li:not(.facet-truncated)")
+        lis = soup.select("#facet-neighborhood-b352a7 ul li:not(.facet-truncated)")
         facet_truncated = soup.select_one(".facet-truncated")
         assert len(lis) == expected_num_facets
         if not expected_ellipses:

tests/test_internals_database.py

@@ -82,7 +82,7 @@ async def test_table_exists(db, tables, exists):
                 "on_earth",
                 "state",
                 "city_id",
-                "neighborhood",
+                "_neighborhood",
                 "tags",
                 "complex_array",
                 "distinct_some_null",
@@ -170,7 +170,7 @@ async def test_table_columns(db, table, expected):
                 ),
                 Column(
                     cid=6,
-                    name="neighborhood",
+                    name="_neighborhood",
                     type="text",
                     notnull=0,
                     default_value=None,

tests/test_plugins.py

@@ -437,7 +437,7 @@ def test_hook_register_output_renderer_all_parameters(app_client):
             "on_earth",
             "state",
             "city_id",
-            "neighborhood",
+            "_neighborhood",
             "tags",
             "complex_array",
             "distinct_some_null",
@@ -459,7 +459,7 @@ def test_hook_register_output_renderer_all_parameters(app_client):
             "<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 from facetable order by pk limit 51",
         "query_name": None,
         "database": "fixtures",
         "table": "facetable",
@@ -526,12 +526,12 @@ def test_hook_register_output_renderer_can_render(app_client):
             "on_earth",
             "state",
             "city_id",
-            "neighborhood",
+            "_neighborhood",
             "tags",
             "complex_array",
             "distinct_some_null",
         ],
-        "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 from facetable order by pk limit 51",
         "query_name": None,
         "database": "fixtures",
         "table": "facetable",