Skip to content

Show referring tables and rows when the referring foreign key is compound #2003

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+182 −120
Open

Show referring tables and rows when the referring foreign key is compound #2003

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41beef0
spike to fix half of #1099
fgregg Jan 24, 2023
1e842ef
more narrowing to singletons
fgregg Jan 24, 2023
36fa32d
getting tests working
fgregg Jan 25, 2023
1e5b42f
adjust tests
fgregg Jan 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion datasette/app.py
View file
Original file line number Diff line number Diff line change
@@ -899,8 +899,11 @@ async def expand_foreign_keys(self, database, table, column, values):
fk = [
foreign_key
for foreign_key in foreign_keys
if foreign_key["column"] == column
if foreign_key["columns"][0] == column
Copy link
Contributor Author

@fgregg fgregg Jan 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we don't have a strong idea of how to display compound foreign keys in a table view, we have to do this operation in a few places.

and len(foreign_key["columns"]) == 1
][0]
fk["column"] = fk["columns"][0]
fk["other_column"] = fk["other_columns"][0]
except IndexError:
return {}
label_column = await db.label_column_for_table(fk["other_table"])
6 changes: 5 additions & 1 deletion datasette/filters.py
View file
Original file line number Diff line number Diff line change
@@ -135,8 +135,12 @@ async def inner():
outgoing_foreign_keys = await db.foreign_keys_for_table(through_table)
try:
fk_to_us = [
fk for fk in outgoing_foreign_keys if fk["other_table"] == table
fk
for fk in outgoing_foreign_keys
if fk["other_table"] == table and len(fk["other_columns"]) == 1
][0]
fk_to_us["column"] = fk_to_us["columns"][0]
fk_to_us["other_column"] = fk_to_us["other_columns"][0]
except IndexError:
raise DatasetteError(
"Invalid _through - could not find corresponding foreign key"
2 changes: 1 addition & 1 deletion datasette/templates/row.html
View file
Original file line number Diff line number Diff line change
@@ -35,7 +35,7 @@ <h2>Links from other tables</h2>
<li>
<a href="{{ other.link }}">
{{ "{:,}".format(other.count) }} row{% if other.count == 1 %}{% else %}s{% endif %}</a>
from {{ other.other_column }} in {{ other.other_table }}
from {{ other.other_columns_reference }} in {{ other.other_table }}
</li>
{% endfor %}
</ul>
37 changes: 14 additions & 23 deletions datasette/utils/__init__.py
View file
Original file line number Diff line number Diff line change
@@ -523,30 +523,21 @@ def detect_primary_keys(conn, table):

def get_outbound_foreign_keys(conn, table):
infos = conn.execute(f"PRAGMA foreign_key_list([{table}])").fetchall()
fks = []
fks = {}
for info in infos:
if info is not None:
id, seq, table_name, from_, to_, on_update, on_delete, match = info
fks.append(
{
"column": from_,
if id in fks:
fk_info = fks[id]
fk_info["columns"] += (from_,)
fk_info["other_columns"] += (to_,)
else:
fks[id] = {
"other_table": table_name,
"other_column": to_,
"id": id,
"seq": seq,
"columns": (from_,),
"other_columns": (to_,),
}
)
# Filter out compound foreign keys by removing any where "id" is not unique
id_counts = Counter(fk["id"] for fk in fks)
return [
{
"column": fk["column"],
"other_table": fk["other_table"],
"other_column": fk["other_column"],
}
for fk in fks
if id_counts[fk["id"]] == 1
]
return list(fks.values())


def get_all_foreign_keys(conn):
@@ -560,17 +551,17 @@ def get_all_foreign_keys(conn):
fks = get_outbound_foreign_keys(conn, table)
for fk in fks:
table_name = fk["other_table"]
from_ = fk["column"]
to_ = fk["other_column"]
from_ = fk["columns"]
to_ = fk["other_columns"]
if table_name not in table_to_foreign_keys:
# Weird edge case where something refers to a table that does
# not actually exist
continue
table_to_foreign_keys[table_name]["incoming"].append(
{"other_table": table, "column": to_, "other_column": from_}
{"other_table": table, "columns": to_, "other_columns": from_}
)
table_to_foreign_keys[table]["outgoing"].append(
{"other_table": table_name, "column": from_, "other_column": to_}
{"other_table": table_name, "columns": from_, "other_columns": to_}
)

return table_to_foreign_keys
48 changes: 33 additions & 15 deletions datasette/views/row.py
View file
Original file line number Diff line number Diff line change
@@ -97,8 +97,6 @@ async def template_data():
)

async def foreign_key_tables(self, database, table, pk_values):
if len(pk_values) != 1:
return []
db = self.ds.databases[database]
all_foreign_keys = await db.get_all_foreign_keys()
foreign_keys = all_foreign_keys[table]["incoming"]
@@ -107,39 +105,59 @@ async def foreign_key_tables(self, database, table, pk_values):

sql = "select " + ", ".join(
[
"(select count(*) from {table} where {column}=:id)".format(
"(select count(*) from {table} where {condition})".format(
table=escape_sqlite(fk["other_table"]),
column=escape_sqlite(fk["other_column"]),
condition=" and ".join(
"{column}=:id{i}".format(column=escape_sqlite(column), i=i)
for i, column in enumerate(fk["other_columns"])
),
)
for fk in foreign_keys
]
)
try:
rows = list(await db.execute(sql, {"id": pk_values[0]}))
rows = list(
await db.execute(
sql, {"id{i}".format(i=i): pk for i, pk in enumerate(pk_values)}
)
)
except QueryInterrupted:
# Almost certainly hit the timeout
return []

foreign_table_counts = dict(
zip(
[(fk["other_table"], fk["other_column"]) for fk in foreign_keys],
[(fk["other_table"], fk["other_columns"]) for fk in foreign_keys],
list(rows[0]),
)
)
foreign_key_tables = []
for fk in foreign_keys:
count = (
foreign_table_counts.get((fk["other_table"], fk["other_column"])) or 0
foreign_table_counts.get((fk["other_table"], fk["other_columns"])) or 0
)
query_pairs = zip(fk["other_columns"], pk_values)
query = "&".join(
"{}={}".format(col + "__exact" if col.startswith("_") else col, pk)
for col, pk in query_pairs
)
link = "{}?{}".format(
self.ds.urls.table(database, fk["other_table"]), query
)
key = fk["other_column"]
if key.startswith("_"):
key += "__exact"
link = "{}?{}={}".format(
self.ds.urls.table(database, fk["other_table"]),
key,
",".join(pk_values),
if len(pk_values) == 1:
other_columns_reference = fk["other_columns"][0]
else:
other_columns_reference = "({})".format(", ".join(fk["other_columns"]))
foreign_key_tables.append(
{
**fk,
**{
"count": count,
"link": link,
"other_columns_reference": other_columns_reference,
},
}
)
foreign_key_tables.append({**fk, **{"count": count, "link": link}})
return foreign_key_tables


12 changes: 10 additions & 2 deletions datasette/views/table.py
View file
Original file line number Diff line number Diff line change
@@ -88,8 +88,15 @@ async def expandable_columns(self, database_name, table_name):
expandables = []
db = self.ds.databases[database_name]
for fk in await db.foreign_keys_for_table(table_name):
if len(fk["other_columns"]) > 1:
continue
label_column = await db.label_column_for_table(fk["other_table"])
expandables.append((fk, label_column))
singleton_fk = {
"other_table": fk["other_table"],
"other_column": fk["other_columns"][0],
"column": fk["columns"][0],
}
expandables.append((singleton_fk, label_column))
return expandables

async def post(self, request):
@@ -920,8 +927,9 @@ async def display_columns_and_rows(
)

column_to_foreign_key_table = {
fk["column"]: fk["other_table"]
fk["columns"][0]: fk["other_table"]
for fk in await db.foreign_keys_for_table(table_name)
if len(fk["columns"]) == 1
}

cell_rows = []
151 changes: 88 additions & 63 deletions tests/test_api.py
View file
Original file line number Diff line number Diff line change
@@ -96,8 +96,8 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "roadside_attraction_characteristics",
"column": "pk",
"other_column": "characteristic_id",
"columns": ["pk"],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is how the API changes.

"other_columns": ["characteristic_id"],
}
],
"outgoing": [],
@@ -126,18 +126,18 @@ async def test_database_page(ds_client):
"outgoing": [
{
"other_table": "simple_primary_key",
"column": "f3",
"other_column": "id",
"columns": ["f3"],
"other_columns": ["id"],
},
{
"other_table": "simple_primary_key",
"column": "f2",
"other_column": "id",
"columns": ["f2"],
"other_columns": ["id"],
},
{
"other_table": "simple_primary_key",
"column": "f1",
"other_column": "id",
"columns": ["f1"],
"other_columns": ["id"],
},
],
},
@@ -150,7 +150,19 @@ async def test_database_page(ds_client):
"count": 2,
"hidden": False,
"fts_table": None,
"foreign_keys": {"incoming": [], "outgoing": []},
"foreign_keys": {
"incoming": [
{
"other_table": "foreign_key_references",
"columns": ["pk1", "pk2"],
"other_columns": [
"foreign_key_compound_pk1",
"foreign_key_compound_pk2",
],
}
],
"outgoing": [],
},
"private": False,
},
{
@@ -175,8 +187,8 @@ async def test_database_page(ds_client):
"outgoing": [
{
"other_table": "primary_key_multiple_columns_explicit_label",
"column": "foreign_key_with_custom_label",
"other_column": "id",
"columns": ["foreign_key_with_custom_label"],
"other_columns": ["id"],
}
],
},
@@ -193,8 +205,8 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "facetable",
"column": "id",
"other_column": "_city_id",
"columns": ["id"],
"other_columns": ["_city_id"],
}
],
"outgoing": [],
@@ -225,8 +237,8 @@ async def test_database_page(ds_client):
"outgoing": [
{
"other_table": "facet_cities",
"column": "_city_id",
"other_column": "id",
"columns": ["_city_id"],
"other_columns": ["id"],
}
],
},
@@ -249,20 +261,28 @@ async def test_database_page(ds_client):
"foreign_keys": {
"incoming": [],
"outgoing": [
{
"other_table": "compound_primary_key",
"columns": [
"foreign_key_compound_pk1",
"foreign_key_compound_pk2",
],
"other_columns": ["pk1", "pk2"],
},
{
"other_table": "primary_key_multiple_columns",
"column": "foreign_key_with_no_label",
"other_column": "id",
"columns": ["foreign_key_with_no_label"],
"other_columns": ["id"],
},
{
"other_table": "simple_primary_key",
"column": "foreign_key_with_blank_label",
"other_column": "id",
"columns": ["foreign_key_with_blank_label"],
"other_columns": ["id"],
},
{
"other_table": "simple_primary_key",
"column": "foreign_key_with_label",
"other_column": "id",
"columns": ["foreign_key_with_label"],
"other_columns": ["id"],
},
],
},
@@ -290,8 +310,8 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "foreign_key_references",
"column": "id",
"other_column": "foreign_key_with_no_label",
"columns": ["id"],
"other_columns": ["foreign_key_with_no_label"],
}
],
"outgoing": [],
@@ -309,8 +329,8 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "custom_foreign_key_label",
"column": "id",
"other_column": "foreign_key_with_custom_label",
"columns": ["id"],
"other_columns": ["foreign_key_with_custom_label"],
}
],
"outgoing": [],
@@ -329,13 +349,13 @@ async def test_database_page(ds_client):
"outgoing": [
{
"other_table": "attraction_characteristic",
"column": "characteristic_id",
"other_column": "pk",
"columns": ["characteristic_id"],
"other_columns": ["pk"],
},
{
"other_table": "roadside_attractions",
"column": "attraction_id",
"other_column": "pk",
"columns": ["attraction_id"],
"other_columns": ["pk"],
},
],
},
@@ -352,8 +372,8 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "roadside_attraction_characteristics",
"column": "pk",
"other_column": "attraction_id",
"columns": ["pk"],
"other_columns": ["attraction_id"],
}
],
"outgoing": [],
@@ -371,8 +391,8 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "searchable_tags",
"column": "pk",
"other_column": "searchable_id",
"columns": ["pk"],
"other_columns": ["searchable_id"],
}
],
"outgoing": [],
@@ -389,11 +409,15 @@ async def test_database_page(ds_client):
"foreign_keys": {
"incoming": [],
"outgoing": [
{"other_table": "tags", "column": "tag", "other_column": "tag"},
{
"other_table": "tags",
"columns": ["tag"],
"other_columns": ["tag"],
},
{
"other_table": "searchable",
"column": "searchable_id",
"other_column": "pk",
"columns": ["searchable_id"],
"other_columns": ["pk"],
},
],
},
@@ -420,28 +444,28 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "foreign_key_references",
"column": "id",
"other_column": "foreign_key_with_blank_label",
"columns": ["id"],
"other_columns": ["foreign_key_with_blank_label"],
},
{
"other_table": "foreign_key_references",
"column": "id",
"other_column": "foreign_key_with_label",
"columns": ["id"],
"other_columns": ["foreign_key_with_label"],
},
{
"other_table": "complex_foreign_keys",
"column": "id",
"other_column": "f3",
"columns": ["id"],
"other_columns": ["f3"],
},
{
"other_table": "complex_foreign_keys",
"column": "id",
"other_column": "f2",
"columns": ["id"],
"other_columns": ["f2"],
},
{
"other_table": "complex_foreign_keys",
"column": "id",
"other_column": "f1",
"columns": ["id"],
"other_columns": ["f1"],
},
],
"outgoing": [],
@@ -487,8 +511,8 @@ async def test_database_page(ds_client):
"incoming": [
{
"other_table": "searchable_tags",
"column": "tag",
"other_column": "tag",
"columns": ["tag"],
"other_columns": ["tag"],
}
],
"outgoing": [],
@@ -517,11 +541,7 @@ async def test_database_page(ds_client):
},
{
"name": "searchable_fts",
"columns": [
"text1",
"text2",
"name with . and spaces",
]
"columns": ["text1", "text2", "name with . and spaces"]
+ (
[
"searchable_fts",
@@ -720,38 +740,43 @@ async def test_row_foreign_key_tables(ds_client):
assert response.json()["foreign_key_tables"] == [
{
"other_table": "foreign_key_references",
"column": "id",
"other_column": "foreign_key_with_blank_label",
"columns": ["id"],
"other_columns": ["foreign_key_with_blank_label"],
"count": 0,
"link": "/fixtures/foreign_key_references?foreign_key_with_blank_label=1",
"other_columns_reference": "foreign_key_with_blank_label",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also another API change. this got introduced so we could have compound keys look good in the row template, but we could do that through a custom filter instead.

},
{
"other_table": "foreign_key_references",
"column": "id",
"other_column": "foreign_key_with_label",
"columns": ["id"],
"other_columns": ["foreign_key_with_label"],
"count": 1,
"link": "/fixtures/foreign_key_references?foreign_key_with_label=1",
"other_columns_reference": "foreign_key_with_label",
},
{
"other_table": "complex_foreign_keys",
"column": "id",
"other_column": "f3",
"columns": ["id"],
"other_columns": ["f3"],
"count": 1,
"link": "/fixtures/complex_foreign_keys?f3=1",
"other_columns_reference": "f3",
},
{
"other_table": "complex_foreign_keys",
"column": "id",
"other_column": "f2",
"columns": ["id"],
"other_columns": ["f2"],
"count": 0,
"link": "/fixtures/complex_foreign_keys?f2=1",
"other_columns_reference": "f2",
},
{
"other_table": "complex_foreign_keys",
"column": "id",
"other_column": "f1",
"columns": ["id"],
"other_columns": ["f1"],
"count": 1,
"link": "/fixtures/complex_foreign_keys?f1=1",
"other_columns_reference": "f1",
},
]

41 changes: 27 additions & 14 deletions tests/test_internals_database.py
View file
Original file line number Diff line number Diff line change
@@ -329,48 +329,61 @@ async def test_get_all_foreign_keys(db):
"outgoing": [
{
"other_table": "attraction_characteristic",
"column": "characteristic_id",
"other_column": "pk",
"columns": ("characteristic_id",),
"other_columns": ("pk",),
},
{
"other_table": "roadside_attractions",
"column": "attraction_id",
"other_column": "pk",
"columns": ("attraction_id",),
"other_columns": ("pk",),
},
],
}
assert all_foreign_keys["attraction_characteristic"] == {
"incoming": [
{
"other_table": "roadside_attraction_characteristics",
"column": "pk",
"other_column": "characteristic_id",
"columns": ("pk",),
"other_columns": ("characteristic_id",),
}
],
"outgoing": [],
}
assert all_foreign_keys["compound_primary_key"] == {
# No incoming because these are compound foreign keys, which we currently ignore
"incoming": [],
"incoming": [
{
"other_table": "foreign_key_references",
"columns": ("pk1", "pk2"),
"other_columns": (
"foreign_key_compound_pk1",
"foreign_key_compound_pk2",
),
}
],
"outgoing": [],
}
assert all_foreign_keys["foreign_key_references"] == {
"incoming": [],
"outgoing": [
{
"other_table": "compound_primary_key",
"columns": ("foreign_key_compound_pk1", "foreign_key_compound_pk2"),
"other_columns": ("pk1", "pk2"),
},
{
"other_table": "primary_key_multiple_columns",
"column": "foreign_key_with_no_label",
"other_column": "id",
"columns": ("foreign_key_with_no_label",),
"other_columns": ("id",),
},
{
"other_table": "simple_primary_key",
"column": "foreign_key_with_blank_label",
"other_column": "id",
"columns": ("foreign_key_with_blank_label",),
"other_columns": ("id",),
},
{
"other_table": "simple_primary_key",
"column": "foreign_key_with_label",
"other_column": "id",
"columns": ("foreign_key_with_label",),
"other_columns": ("id",),
},
],
}