From c2779e5af0d056ef1637f9f0e191dca421259a5e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 16 Nov 2019 23:10:59 -0800 Subject: [PATCH 1/3] Experimental WIP for #620 Example URL: /fixtures/facetable?_fk.on_earth=facet_cities.id --- datasette/app.py | 4 ++-- datasette/views/table.py | 33 +++++++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 119d0e19..08076896 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -352,11 +352,11 @@ class Datasette: log_sql_errors=log_sql_errors, ) - async def expand_foreign_keys(self, database, table, column, values): + async def expand_foreign_keys(self, database, table, column, values, fks=None): "Returns dict mapping (column, value) -> label" labeled_fks = {} db = self.databases[database] - foreign_keys = await db.foreign_keys_for_table(table) + foreign_keys = fks or await db.foreign_keys_for_table(table) # Find the foreign_key for this column try: fk = [ diff --git a/datasette/views/table.py b/datasette/views/table.py index a60a3941..494b6f29 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -74,17 +74,36 @@ class RowTableShared(DataView): sortable_columns.add("rowid") return sortable_columns - async def expandable_columns(self, database, table): + async def foreign_keys_for_table(self, request, database, table): + db = self.ds.databases[database] + fks = await db.foreign_keys_for_table(table) + # Handle ?_fk.article_id=articles.id querystring arguments + for key, value in request.args.items(): + if key.startswith("_fk."): + value = value[0] + column = key.split("_fk.", 1)[1] + other_table, other_column = value.split(".", 1) + # {'other_table': '...', 'column': '...', 'other_column': 'id'} + expandable_fk = { + "other_table": other_table, + "column": column, + "other_column": other_column, + } + fks.append(expandable_fk) + return fks + + + async def expandable_columns(self, request, database, table): # Returns list of (fk_dict, label_column-or-None) pairs for that table expandables = [] db = self.ds.databases[database] - for fk in await db.foreign_keys_for_table(table): + for fk in await self.foreign_keys_for_table(request, database, table): label_column = await db.label_column_for_table(fk["other_table"]) expandables.append((fk, label_column)) return expandables async def display_columns_and_rows( - self, database, table, description, rows, link_column=False, truncate_cells=0 + self, request, database, table, description, rows, link_column=False, truncate_cells=0 ): "Returns columns, rows for specified table - including fancy foreign key treatment" db = self.ds.databases[database] @@ -96,7 +115,7 @@ class RowTableShared(DataView): pks = await db.primary_keys(table) column_to_foreign_key_table = { fk["column"]: fk["other_table"] - for fk in await db.foreign_keys_for_table(table) + for fk in await self.foreign_keys_for_table(request, database, table) } cell_rows = [] @@ -593,7 +612,7 @@ class TableView(RowTableShared): # Expand labeled columns if requested expanded_columns = [] - expandable_columns = await self.expandable_columns(database, table) + expandable_columns = await self.expandable_columns(request, database, table) columns_to_expand = None try: all_labels = value_as_boolean(special_args.get("_labels", "")) @@ -618,7 +637,7 @@ class TableView(RowTableShared): values = [row[column_index] for row in rows] # Expand them expanded_labels.update( - await self.ds.expand_foreign_keys(database, table, column, values) + await self.ds.expand_foreign_keys(database, table, column, values, fks=[p[0] for p in expandable_columns]) ) if expanded_labels: # Rewrite the rows @@ -693,6 +712,7 @@ class TableView(RowTableShared): async def extra_template(): display_columns, display_rows = await self.display_columns_and_rows( + request, database, table, results.description, @@ -807,6 +827,7 @@ class RowView(RowTableShared): async def template_data(): display_columns, display_rows = await self.display_columns_and_rows( + request, database, table, results.description, From eb61845141ce811a126b2cacd9cd9a76809cab32 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 17 Nov 2019 22:24:55 -0800 Subject: [PATCH 2/3] Format with black --- datasette/views/table.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/datasette/views/table.py b/datasette/views/table.py index 494b6f29..b0ea54d2 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -92,7 +92,6 @@ class RowTableShared(DataView): fks.append(expandable_fk) return fks - async def expandable_columns(self, request, database, table): # Returns list of (fk_dict, label_column-or-None) pairs for that table expandables = [] @@ -103,7 +102,14 @@ class RowTableShared(DataView): return expandables async def display_columns_and_rows( - self, request, database, table, description, rows, link_column=False, truncate_cells=0 + self, + request, + database, + table, + description, + rows, + link_column=False, + truncate_cells=0, ): "Returns columns, rows for specified table - including fancy foreign key treatment" db = self.ds.databases[database] @@ -637,7 +643,13 @@ class TableView(RowTableShared): values = [row[column_index] for row in rows] # Expand them expanded_labels.update( - await self.ds.expand_foreign_keys(database, table, column, values, fks=[p[0] for p in expandable_columns]) + await self.ds.expand_foreign_keys( + database, + table, + column, + values, + fks=[p[0] for p in expandable_columns], + ) ) if expanded_labels: # Rewrite the rows From dcc5cd425a4e6608d8f43b6df8ec689790ad775f Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 17 Nov 2019 22:33:43 -0800 Subject: [PATCH 3/3] Moved expand_foreign_keys method to Database Refs #620 Also fixed a few places that were calling ds.execute() instead of db.execute() --- datasette/app.py | 37 ------------------------------------- datasette/database.py | 37 +++++++++++++++++++++++++++++++++++++ datasette/facets.py | 12 ++++++------ datasette/views/table.py | 20 ++++++-------------- 4 files changed, 49 insertions(+), 57 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 08076896..cadeae14 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -352,43 +352,6 @@ class Datasette: log_sql_errors=log_sql_errors, ) - async def expand_foreign_keys(self, database, table, column, values, fks=None): - "Returns dict mapping (column, value) -> label" - labeled_fks = {} - db = self.databases[database] - foreign_keys = fks or await db.foreign_keys_for_table(table) - # Find the foreign_key for this column - try: - fk = [ - foreign_key - for foreign_key in foreign_keys - if foreign_key["column"] == column - ][0] - except IndexError: - return {} - label_column = await db.label_column_for_table(fk["other_table"]) - if not label_column: - return {(fk["column"], value): str(value) for value in values} - labeled_fks = {} - sql = """ - select {other_column}, {label_column} - from {other_table} - where {other_column} in ({placeholders}) - """.format( - other_column=escape_sqlite(fk["other_column"]), - label_column=escape_sqlite(label_column), - other_table=escape_sqlite(fk["other_table"]), - placeholders=", ".join(["?"] * len(set(values))), - ) - try: - results = await self.execute(database, sql, list(set(values))) - except QueryInterrupted: - pass - else: - for id, value in results: - labeled_fks[(fk["column"], id)] = value - return labeled_fks - def absolute_url(self, request, path): url = urllib.parse.urljoin(request.url, path) if url.startswith("http://") and self.config("force_https_urls"): diff --git a/datasette/database.py b/datasette/database.py index 9a8ae4d4..53b5cd5c 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -10,6 +10,7 @@ from .utils import ( detect_fts, detect_primary_keys, detect_spatialite, + escape_sqlite, get_all_foreign_keys, get_outbound_foreign_keys, sqlite_timelimit, @@ -217,6 +218,42 @@ class Database: lambda conn: get_outbound_foreign_keys(conn, table) ) + async def expand_foreign_keys(self, table, column, values, fks=None): + "Returns dict mapping (column, value) -> label" + labeled_fks = {} + foreign_keys = fks or await self.foreign_keys_for_table(table) + # Find the foreign_key for this column + try: + fk = [ + foreign_key + for foreign_key in foreign_keys + if foreign_key["column"] == column + ][0] + except IndexError: + return {} + label_column = await self.label_column_for_table(fk["other_table"]) + if not label_column: + return {(fk["column"], value): str(value) for value in values} + labeled_fks = {} + sql = """ + select {other_column}, {label_column} + from {other_table} + where {other_column} in ({placeholders}) + """.format( + other_column=escape_sqlite(fk["other_column"]), + label_column=escape_sqlite(label_column), + other_table=escape_sqlite(fk["other_table"]), + placeholders=", ".join(["?"] * len(set(values))), + ) + try: + results = await self.execute(sql, list(set(values))) + except QueryInterrupted: + pass + else: + for id, value in results: + labeled_fks[(fk["column"], id)] = value + return labeled_fks + async def hidden_table_names(self): # Mark tables 'hidden' if they relate to FTS virtual tables hidden_tables = [ diff --git a/datasette/facets.py b/datasette/facets.py index 0c6459d6..8c97e36c 100644 --- a/datasette/facets.py +++ b/datasette/facets.py @@ -139,6 +139,7 @@ class ColumnFacet(Facet): facet_size = self.ds.config("default_facet_size") suggested_facets = [] already_enabled = [c["config"]["simple"] for c in self.get_configs()] + database = self.ds.databases[self.database] for column in columns: if column in already_enabled: continue @@ -152,8 +153,7 @@ class ColumnFacet(Facet): ) distinct_values = None try: - distinct_values = await self.ds.execute( - self.database, + distinct_values = await database.execute( suggested_facet_sql, self.params, truncate=False, @@ -182,6 +182,7 @@ class ColumnFacet(Facet): async def facet_results(self): facet_results = {} facets_timed_out = [] + database = self.ds.databases[self.database] qs_pairs = self.get_querystring_pairs() @@ -200,8 +201,7 @@ class ColumnFacet(Facet): col=escape_sqlite(column), sql=self.sql, limit=facet_size + 1 ) try: - facet_rows_results = await self.ds.execute( - self.database, + facet_rows_results = await database.execute( facet_sql, self.params, truncate=False, @@ -222,8 +222,8 @@ class ColumnFacet(Facet): if self.table: # Attempt to expand foreign keys into labels values = [row["value"] for row in facet_rows] - expanded = await self.ds.expand_foreign_keys( - self.database, self.table, column, values + expanded = await database.expand_foreign_keys( + self.table, column, values ) else: expanded = {} diff --git a/datasette/views/table.py b/datasette/views/table.py index b0ea54d2..4cc19d58 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -558,17 +558,13 @@ class TableView(RowTableShared): if request.raw_args.get("_timelimit"): extra_args["custom_time_limit"] = int(request.raw_args["_timelimit"]) - results = await self.ds.execute( - database, sql, params, truncate=True, **extra_args - ) + results = await db.execute(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) - ) + count_rows = list(await db.execute(count_sql, from_sql_params)) filtered_table_rows_count = count_rows[0][0] except QueryInterrupted: pass @@ -643,12 +639,8 @@ class TableView(RowTableShared): values = [row[column_index] for row in rows] # Expand them expanded_labels.update( - await self.ds.expand_foreign_keys( - database, - table, - column, - values, - fks=[p[0] for p in expandable_columns], + await db.expand_foreign_keys( + table, column, values, fks=[p[0] for p in expandable_columns], ) ) if expanded_labels: @@ -831,7 +823,7 @@ class RowView(RowTableShared): params = {} for i, pk_value in enumerate(pk_values): params["p{}".format(i)] = pk_value - results = await self.ds.execute(database, sql, params, truncate=True) + results = await db.execute(sql, params, truncate=True) columns = [r[0] for r in results.description] rows = list(results.rows) if not rows: @@ -913,7 +905,7 @@ class RowView(RowTableShared): ] ) try: - rows = list(await self.ds.execute(database, sql, {"id": pk_values[0]})) + rows = list(await db.execute(sql, {"id": pk_values[0]})) except sqlite3.OperationalError: # Almost certainly hit the timeout return []