Facets now execute ignoring ?_col and ?_nocol, fixes #1345

simonw · Jun 2, 2021 · 0d6afc8 · 0d6afc8
1 parent d337e34
commit 0d6afc8
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 8 deletions.
diff --git a/datasette/views/table.py b/datasette/views/table.py
@@ -358,16 +358,21 @@ async def data(
         )
 
         pks = await db.primary_keys(table)
-        table_columns = await self.columns_to_select(db, table, request)
-        select_clause = ", ".join(escape_sqlite(t) for t in table_columns)
+        table_columns = await db.table_columns(table)
+
+        specified_columns = await self.columns_to_select(db, table, request)
+        select_specified_columns = ", ".join(
+            escape_sqlite(t) for t in specified_columns
+        )
+        select_all_columns = ", ".join(escape_sqlite(t) for t in table_columns)
 
         use_rowid = not pks and not is_view
         if use_rowid:
-            select = f"rowid, {select_clause}"
+            select_specified_columns = f"rowid, {select_specified_columns}"
+            select_all_columns = f"rowid, {select_all_columns}"
             order_by = "rowid"
             order_by_pks = "rowid"
         else:
-            select = select_clause
             order_by_pks = ", ".join([escape_sqlite(pk) for pk in pks])
             order_by = order_by_pks
 
@@ -633,7 +638,7 @@ async def data(
             where_clause = f"where {' and '.join(where_clauses)} "
 
         if order_by:
-            order_by = f"order by {order_by} "
+            order_by = f"order by {order_by}"
 
         extra_args = {}
         # Handle ?_size=500
@@ -656,13 +661,22 @@ async def data(
         else:
             page_size = self.ds.page_size
 
-        sql_no_limit = "select {select} from {table_name} {where}{order_by}".format(
-            select=select,
+        sql_no_limit = (
+            "select {select_all_columns} from {table_name} {where}{order_by}".format(
+                select_all_columns=select_all_columns,
+                table_name=escape_sqlite(table),
+                where=where_clause,
+                order_by=order_by,
+            )
+        )
+        sql = "select {select_specified_columns} from {table_name} {where}{order_by} limit {page_size}{offset}".format(
+            select_specified_columns=select_specified_columns,
             table_name=escape_sqlite(table),
             where=where_clause,
             order_by=order_by,
+            page_size=page_size + 1,
+            offset=offset,
         )
-        sql = f"{sql_no_limit.rstrip()} limit {page_size + 1}{offset}"
 
         if request.args.get("_timelimit"):
             extra_args["custom_time_limit"] = int(request.args.get("_timelimit"))

diff --git a/tests/test_api.py b/tests/test_api.py
@@ -2041,6 +2041,21 @@ def test_http_options_request(app_client):
             "/fixtures/facetable.json?_col=state&_col=created&_nocol=created",
             ["pk", "state"],
         ),
+        (
+            # Ensure faceting doesn't break, https://github.com/simonw/datasette/issues/1345
+            "/fixtures/facetable.json?_nocol=state&_facet=state",
+            [
+                "pk",
+                "created",
+                "planet_int",
+                "on_earth",
+                "city_id",
+                "neighborhood",
+                "tags",
+                "complex_array",
+                "distinct_some_null",
+            ],
+        ),
         (
             "/fixtures/simple_view.json?_nocol=content",
             ["upper_content"],