Skip to content

Commit

Permalink
expand_foreign_keys() no longer uses inspect, refs #420
Browse files Browse the repository at this point in the history
  • Loading branch information
simonw committed Apr 7, 2019
1 parent 97331f3 commit 53bf875
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 63 deletions.
29 changes: 29 additions & 0 deletions datasette/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
Results,
escape_css_string,
escape_sqlite,
get_outbound_foreign_keys,
get_plugins,
module_from_path,
sqlite3,
Expand Down Expand Up @@ -464,11 +465,39 @@ def plugins(self, show_all=False):
for p in ps
]

def table_metadata(self, database, table):
"Fetch table-specific metadata."
return (self.metadata("databases") or {}).get(database, {}).get(
"tables", {}
).get(
table, {}
)

async def table_columns(self, db_name, table):
return await self.execute_against_connection_in_thread(
db_name, lambda conn: table_columns(conn, table)
)

async def foreign_keys_for_table(self, database, table):
return await self.execute_against_connection_in_thread(
database, lambda conn: get_outbound_foreign_keys(conn, table)
)

async def label_column_for_table(self, db_name, table):
explicit_label_column = (
self.table_metadata(
db_name, table
).get("label_column")
)
if explicit_label_column:
return explicit_label_column
# If a table has two columns, one of which is ID, then label_column is the other one
column_names = await self.table_columns(db_name, table)
if (column_names and len(column_names) == 2 and "id" in column_names):
return [c for c in column_names if c != "id"][0]
# Couldn't find a label:
return None

async def execute_against_connection_in_thread(self, db_name, fn):
def in_thread():
conn = getattr(connections, db_name, None)
Expand Down
12 changes: 0 additions & 12 deletions datasette/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,6 @@ def inspect_views(conn):
return [v[0] for v in conn.execute('select name from sqlite_master where type = "view"')]


def detect_label_column(column_names):
""" Detect the label column - which we display as the label for a joined column.
If a table has two columns, one of which is ID, then label_column is the other one.
"""
if (column_names and len(column_names) == 2 and "id" in column_names):
return [c for c in column_names if c != "id"][0]

return None


def detect_primary_keys(conn, table):
" Figure out primary keys for a table. "
table_info_rows = [
Expand Down Expand Up @@ -86,7 +75,6 @@ def inspect_tables(conn, database_metadata):
"columns": column_names,
"primary_keys": detect_primary_keys(conn, table),
"count": count,
"label_column": detect_label_column(column_names),
"hidden": table_metadata.get("hidden") or False,
"fts_table": detect_fts(conn, table),
}
Expand Down
16 changes: 16 additions & 0 deletions datasette/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,22 @@ def temporary_heroku_directory(
os.chdir(saved_cwd)


def get_outbound_foreign_keys(conn, table):
infos = conn.execute(
'PRAGMA foreign_key_list([{}])'.format(table)
).fetchall()
fks = []
for info in infos:
if info is not None:
id, seq, table_name, from_, to_, on_update, on_delete, match = info
fks.append({
'other_table': table_name,
'column': from_,
'other_column': to_
})
return fks


def get_all_foreign_keys(conn):
tables = [r[0] for r in conn.execute('select name from sqlite_master where type="table"')]
table_to_foreign_keys = {}
Expand Down
8 changes: 0 additions & 8 deletions datasette/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,6 @@ class BaseView(RenderMixin):
def __init__(self, datasette):
self.ds = datasette

def table_metadata(self, database, table):
"Fetch table-specific metadata."
return (self.ds.metadata("databases") or {}).get(database, {}).get(
"tables", {}
).get(
table, {}
)

def options(self, request, *args, **kwargs):
r = response.text("ok")
if self.ds.cors:
Expand Down
24 changes: 7 additions & 17 deletions datasette/views/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
class RowTableShared(BaseView):

async def sortable_columns_for_table(self, database, table, use_rowid):
table_metadata = self.table_metadata(database, table)
table_metadata = self.ds.table_metadata(database, table)
if "sortable_columns" in table_metadata:
sortable_columns = set(table_metadata["sortable_columns"])
else:
Expand All @@ -51,7 +51,7 @@ def expandable_columns(self, database, table):
expandables = []
for fk in table_info["foreign_keys"]["outgoing"]:
label_column = (
self.table_metadata(
self.ds.table_metadata(
database, fk["other_table"]
).get("label_column")
or tables.get(fk["other_table"], {}).get("label_column")
Expand All @@ -62,11 +62,7 @@ def expandable_columns(self, database, table):
async def expand_foreign_keys(self, database, table, column, values):
"Returns dict mapping (column, value) -> label"
labeled_fks = {}
tables_info = self.ds.inspect()[database]["tables"]
table_info = tables_info.get(table) or {}
if not table_info:
return {}
foreign_keys = table_info["foreign_keys"]["outgoing"]
foreign_keys = await self.ds.foreign_keys_for_table(database, table)
# Find the foreign_key for this column
try:
fk = [
Expand All @@ -75,13 +71,7 @@ async def expand_foreign_keys(self, database, table, column, values):
][0]
except IndexError:
return {}
label_column = (
# First look in metadata.json for this foreign key table:
self.table_metadata(
database, fk["other_table"]
).get("label_column")
or tables_info.get(fk["other_table"], {}).get("label_column")
)
label_column = await self.ds.label_column_for_table(database, fk["other_table"])
if not label_column:
return {
(fk["column"], value): str(value)
Expand Down Expand Up @@ -119,7 +109,7 @@ async def display_columns_and_rows(
truncate_cells=0,
):
"Returns columns, rows for specified table - including fancy foreign key treatment"
table_metadata = self.table_metadata(database, table)
table_metadata = self.ds.table_metadata(database, table)
info = self.ds.inspect()[database]
sortable_columns = await self.sortable_columns_for_table(database, table, True)
columns = [
Expand Down Expand Up @@ -309,7 +299,7 @@ async def data(self, request, database, hash, table, default_labels=False, _nex
forward_querystring=False,
)

table_metadata = self.table_metadata(database, table)
table_metadata = self.ds.table_metadata(database, table)
units = table_metadata.get("units", {})
filters = Filters(sorted(other_args.items()), units, ureg)
where_clauses, params = filters.build_where_clauses()
Expand Down Expand Up @@ -880,7 +870,7 @@ async def template_data():
"columns": columns,
"primary_keys": pks,
"primary_key_values": pk_values,
"units": self.table_metadata(database, table).get("units", {}),
"units": self.ds.table_metadata(database, table).get("units", {}),
}

if "foreign_key_tables" in (request.raw_args.get("_extras") or "").split(","):
Expand Down
1 change: 0 additions & 1 deletion docs/introspection.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ This is an internal implementation detail of Datasette and the format should not
},
"fts_table": null,
"hidden": false,
"label_column": null,
"name": "./index",
"primary_keys": []
},
Expand Down
25 changes: 0 additions & 25 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def test_database_page(app_client):
'count': 0,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': [],
}, {
Expand All @@ -44,7 +43,6 @@ def test_database_page(app_client):
'count': 0,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk'],
}, {
Expand All @@ -68,7 +66,6 @@ def test_database_page(app_client):
}],
},
'hidden': False,
'label_column': None,
'fts_table': None,
'primary_keys': ['pk'],
}, {
Expand All @@ -77,7 +74,6 @@ def test_database_page(app_client):
'count': 1,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk1', 'pk2'],
}, {
Expand All @@ -86,7 +82,6 @@ def test_database_page(app_client):
'count': 1001,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk1', 'pk2', 'pk3'],
}, {
Expand All @@ -102,7 +97,6 @@ def test_database_page(app_client):
'other_table': 'primary_key_multiple_columns_explicit_label'
}],
},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk'],
}, {
Expand All @@ -119,7 +113,6 @@ def test_database_page(app_client):
},
'fts_table': None,
'hidden': False,
'label_column': 'name',
'primary_keys': ['id'],
}, {
'columns': ['pk', 'planet_int', 'on_earth', 'state', 'city_id', 'neighborhood'],
Expand All @@ -135,7 +128,6 @@ def test_database_page(app_client):
},
'fts_table': None,
'hidden': False,
'label_column': None,
'primary_keys': ['pk'],
}, {
'columns': ['pk', 'foreign_key_with_label', 'foreign_key_with_no_label'],
Expand All @@ -154,15 +146,13 @@ def test_database_page(app_client):
'other_table': 'simple_primary_key',
}],
},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk'],
}, {
"name": "infinity",
"columns": ["value"],
"count": 3,
"primary_keys": [],
"label_column": None,
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []}
Expand All @@ -179,7 +169,6 @@ def test_database_page(app_client):
'outgoing': []
},
'hidden': False,
'label_column': None,
'fts_table': None,
'primary_keys': ['id']
}, {
Expand All @@ -195,7 +184,6 @@ def test_database_page(app_client):
'outgoing': []
},
'hidden': False,
'label_column': None,
'fts_table': None,
'primary_keys': ['id']
}, {
Expand All @@ -209,14 +197,12 @@ def test_database_page(app_client):
}], 'outgoing': []},
'fts_table': 'searchable_fts',
'hidden': False,
'label_column': None,
'primary_keys': ['pk'],
}, {
"name": "searchable_tags",
"columns": ["searchable_id", "tag"],
"primary_keys": ["searchable_id", "tag"],
"count": 2,
"label_column": None,
"hidden": False,
"fts_table": None,
"foreign_keys": {
Expand All @@ -240,7 +226,6 @@ def test_database_page(app_client):
'count': 1,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': [],
}, {
Expand Down Expand Up @@ -268,7 +253,6 @@ def test_database_page(app_client):
}],
'outgoing': [],
},
'label_column': 'content',
'fts_table': None,
'primary_keys': ['id'],
}, {
Expand All @@ -280,7 +264,6 @@ def test_database_page(app_client):
'count': 201,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk1', 'pk2'],
}, {
Expand All @@ -289,15 +272,13 @@ def test_database_page(app_client):
'count': 1,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk'],
}, {
"name": "tags",
"columns": ["tag"],
"primary_keys": ["tag"],
"count": 2,
"label_column": None,
"hidden": False,
"fts_table": None,
"foreign_keys": {
Expand All @@ -316,7 +297,6 @@ def test_database_page(app_client):
'count': 3,
'hidden': False,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': ['pk'],
}, {
Expand All @@ -325,7 +305,6 @@ def test_database_page(app_client):
'count': 201,
'hidden': True,
'foreign_keys': {'incoming': [], 'outgoing': []},
'label_column': None,
'fts_table': None,
'primary_keys': [],
}, {
Expand All @@ -334,7 +313,6 @@ def test_database_page(app_client):
'foreign_keys': {'incoming': [], 'outgoing': []},
'fts_table': 'searchable_fts',
'hidden': True,
'label_column': None,
'name': 'searchable_fts',
'primary_keys': []
}, {
Expand All @@ -343,7 +321,6 @@ def test_database_page(app_client):
'foreign_keys': {'incoming': [], 'outgoing': []},
'fts_table': None,
'hidden': True,
'label_column': None,
'name': 'searchable_fts_content',
'primary_keys': ['docid']
}, {
Expand All @@ -355,7 +332,6 @@ def test_database_page(app_client):
'foreign_keys': {'incoming': [], 'outgoing': []},
'fts_table': None,
'hidden': True,
'label_column': None,
'name': 'searchable_fts_segdir',
'primary_keys': ['level', 'idx']
}, {
Expand All @@ -364,7 +340,6 @@ def test_database_page(app_client):
'foreign_keys': {'incoming': [], 'outgoing': []},
'fts_table': None,
'hidden': True,
'label_column': None,
'name': 'searchable_fts_segments',
'primary_keys': ['blockid']
}] == data['tables']
Expand Down

0 comments on commit 53bf875

Please sign in to comment.