diff --git a/datasette/facets.py b/datasette/facets.py index cd8a1822..cb31721e 100644 --- a/datasette/facets.py +++ b/datasette/facets.py @@ -47,24 +47,38 @@ def register_facet_classes(): class Facet: type = None - def __init__(self, ds, request, database, table, configs): + def __init__( + self, + ds, + request, + database, + sql=None, + table=None, + params=None, + configs=None, + row_count=None, + ): + assert table or sql, "Must provide either table= or sql=" self.ds = ds self.request = request self.database = database - self.table = ( - table - ) # For foreign key expansion. Can be None for e.g. canned SQL queries + # For foreign key expansion. Can be None for e.g. canned SQL queries: + self.table = table + self.sql = sql or "select * from [{}]".format(table) + self.params = params or [] self.configs = configs + # row_count can be None, in which case we calculate it ourselves: + self.row_count = row_count def get_querystring_pairs(self): # ?_foo=bar&_foo=2&empty= becomes: # [('_foo', 'bar'), ('_foo', '2'), ('empty', '')] return urllib.parse.parse_qsl(self.request.query_string, keep_blank_values=True) - async def suggest(self, sql, params, filtered_table_rows_count): + async def suggest(self): return [] - async def facet_results(self, sql, params): + async def facet_results(self): # returns ([results], [timed_out]) # TODO: Include "hideable" with each one somehow, which indicates if it was # defined in metadata (in which case you cannot turn it off) @@ -78,12 +92,24 @@ class Facet: ) ).columns + async def get_row_count(self): + if self.row_count is None: + self.row_count = ( + await self.ds.execute( + self.database, + "select count(*) from ({})".format(self.sql), + self.params, + ) + ).rows[0][0] + return self.row_count + class ColumnFacet(Facet): type = "column" - async def suggest(self, sql, params, filtered_table_rows_count): - columns = await self.get_columns(sql, params) + async def suggest(self): + row_count = await self.get_row_count() + columns = await self.get_columns(self.sql, self.params) facet_size = self.ds.config("default_facet_size") suggested_facets = [] for column in columns: @@ -93,14 +119,14 @@ class ColumnFacet(Facet): ) where {column} is not null limit {limit} """.format( - column=escape_sqlite(column), sql=sql, limit=facet_size + 1 + column=escape_sqlite(column), sql=self.sql, limit=facet_size + 1 ) distinct_values = None try: distinct_values = await self.ds.execute( self.database, suggested_facet_sql, - params, + self.params, truncate=False, custom_time_limit=self.ds.config("facet_suggest_time_limit_ms"), ) @@ -109,7 +135,7 @@ class ColumnFacet(Facet): num_distinct_values and num_distinct_values > 1 and num_distinct_values <= facet_size - and num_distinct_values < filtered_table_rows_count + and num_distinct_values < row_count ): suggested_facets.append( { @@ -124,7 +150,7 @@ class ColumnFacet(Facet): continue return suggested_facets - async def facet_results(self, sql, params): + async def facet_results(self): facet_results = {} facets_timed_out = [] @@ -140,13 +166,13 @@ class ColumnFacet(Facet): where {col} is not null group by {col} order by count desc limit {limit} """.format( - col=escape_sqlite(column), sql=sql, limit=facet_size + 1 + col=escape_sqlite(column), sql=self.sql, limit=facet_size + 1 ) try: facet_rows_results = await self.ds.execute( self.database, facet_sql, - params, + self.params, truncate=False, custom_time_limit=self.ds.config("facet_time_limit_ms"), ) diff --git a/datasette/views/table.py b/datasette/views/table.py index 8dc83cbe..06a22204 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -482,6 +482,17 @@ class TableView(RowTableShared): database, sql, params, truncate=True, **extra_args ) + # Number of filtered rows in whole set: + filtered_table_rows_count = None + if count_sql: + try: + count_rows = list(await self.ds.execute( + database, count_sql, from_sql_params + )) + filtered_table_rows_count = count_rows[0][0] + except InterruptedError: + pass + # facets support if not self.ds.config("allow_facet") and any(arg.startswith("_facet") for arg in request.args): raise DatasetteError("_facet= is not allowed", status=400) @@ -495,12 +506,19 @@ class TableView(RowTableShared): facets_timed_out = [] facet_instances = [] for klass in facet_classes: - facet_instances.append(klass(self.ds, request, database, table, configs=facet_configs.get(klass.type))) + facet_instances.append(klass( + self.ds, + request, + database, + sql=sql_no_limit, + params=params, + table=table, + configs=facet_configs.get(klass.type), + row_count=filtered_table_rows_count, + )) for facet in facet_instances: - instance_facet_results, instance_facets_timed_out = await facet.facet_results( - sql_no_limit, params, - ) + instance_facet_results, instance_facets_timed_out = await facet.facet_results() facet_results.update(instance_facet_results) facets_timed_out.extend(instance_facets_timed_out) @@ -588,17 +606,6 @@ class TableView(RowTableShared): ) rows = rows[:page_size] - # Number of filtered rows in whole set: - filtered_table_rows_count = None - if count_sql: - try: - count_rows = list(await self.ds.execute( - database, count_sql, from_sql_params - )) - filtered_table_rows_count = count_rows[0][0] - except InterruptedError: - pass - # Detect suggested facets suggested_facets = [] @@ -606,7 +613,7 @@ class TableView(RowTableShared): for facet in facet_instances: # TODO: ensure facet is not suggested if it is already active # used to use 'if facet_column in facets' for this - suggested_facets.extend(await facet.suggest(sql_no_limit, params, filtered_table_rows_count)) + suggested_facets.extend(await facet.suggest()) # human_description_en combines filters AND search, if provided human_description_en = filters.human_description_en(extra=search_descriptions) diff --git a/tests/test_facets.py b/tests/test_facets.py index d6444342..c13f6529 100644 --- a/tests/test_facets.py +++ b/tests/test_facets.py @@ -8,13 +8,13 @@ import pytest @pytest.mark.asyncio async def test_column_facet_suggest(app_client): facet = ColumnFacet( - app_client.ds, MockRequest("http://localhost/"), "fixtures", "facetable", [] - ) - suggestions = await facet.suggest( - "select * from facetable", - [], - await app_client.ds.table_count("fixtures", "facetable"), + app_client.ds, + MockRequest("http://localhost/"), + database="fixtures", + sql="select * from facetable", + table="facetable", ) + suggestions = await facet.suggest() assert [ {"name": "planet_int", "toggle_url": "http://localhost/?_facet=planet_int"}, {"name": "on_earth", "toggle_url": "http://localhost/?_facet=on_earth"}, @@ -30,16 +30,12 @@ async def test_column_facet_results(app_client): facet = ColumnFacet( app_client.ds, MockRequest("http://localhost/?_facet=city_id"), - "fixtures", - "facetable", - [{"single": "city_id"}], - ) - buckets, timed_out = await facet.facet_results( - """ - select * from facetable - """, - [], + database="fixtures", + sql="select * from facetable", + table="facetable", + configs=[{"single": "city_id"}], ) + buckets, timed_out = await facet.facet_results() assert [] == timed_out assert { "city_id": {