From 6d91d082e0a7b1a275fe72549c7e132382986342 Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Thu, 15 Aug 2024 13:19:22 -0700 Subject: [PATCH] Hide shadow tables, don't hide virtual tables Closes #2296 --- datasette/database.py | 115 +++++++++++++++++++++++-------- tests/test_api.py | 46 ++++++------- tests/test_html.py | 3 +- tests/test_internals_database.py | 49 ++++++++++++- 4 files changed, 161 insertions(+), 52 deletions(-) diff --git a/datasette/database.py b/datasette/database.py index 71c134d1d9..8d51befda7 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -20,6 +20,7 @@ table_columns, table_column_details, ) +from .utils.sqlite import sqlite_version from .inspect import inspect_hash connections = threading.local() @@ -459,22 +460,95 @@ async def foreign_keys_for_table(self, table): ) async def hidden_table_names(self): - # Mark tables 'hidden' if they relate to FTS virtual tables - hidden_tables = [ - r[0] - for r in ( - await self.execute( + hidden_tables = [] + # Add any tables marked as hidden in config + db_config = self.ds.config.get("databases", {}).get(self.name, {}) + if "tables" in db_config: + hidden_tables += [ + t for t in db_config["tables"] if db_config["tables"][t].get("hidden") + ] + + if sqlite_version()[1] >= 37: + hidden_tables += [ + x[0] + for x in await self.execute( + """ + with shadow_tables as ( + select name + from pragma_table_list + where [type] = 'shadow' + order by name + ), + core_tables as ( + select name + from sqlite_master + WHERE name in ('sqlite_stat1', 'sqlite_stat2', 'sqlite_stat3', 'sqlite_stat4') + OR substr(name, 1, 1) == '_' + ), + combined as ( + select name from shadow_tables + union all + select name from core_tables + ) + select name from combined order by 1 + """ + ) + ] + else: + hidden_tables += [ + x[0] + for x in await self.execute( + """ + WITH base AS ( + SELECT name + FROM sqlite_master + WHERE name IN ('sqlite_stat1', 'sqlite_stat2', 'sqlite_stat3', 'sqlite_stat4') + OR substr(name, 1, 1) == '_' + ), + fts_suffixes AS ( + SELECT column1 AS suffix + FROM (VALUES ('_data'), ('_idx'), ('_docsize'), ('_content'), ('_config')) + ), + fts5_names AS ( + SELECT name + FROM sqlite_master + WHERE sql LIKE '%VIRTUAL TABLE%USING FTS%' + ), + fts5_shadow_tables AS ( + SELECT + printf('%s%s', fts5_names.name, fts_suffixes.suffix) AS name + FROM fts5_names + JOIN fts_suffixes + ), + fts3_suffixes AS ( + SELECT column1 AS suffix + FROM (VALUES ('_content'), ('_segdir'), ('_segments'), ('_stat'), ('_docsize')) + ), + fts3_names AS ( + SELECT name + FROM sqlite_master + WHERE sql LIKE '%VIRTUAL TABLE%USING FTS3%' + OR sql LIKE '%VIRTUAL TABLE%USING FTS4%' + ), + fts3_shadow_tables AS ( + SELECT + printf('%s%s', fts3_names.name, fts3_suffixes.suffix) AS name + FROM fts3_names + JOIN fts3_suffixes + ), + final AS ( + SELECT name FROM base + UNION ALL + SELECT name FROM fts5_shadow_tables + UNION ALL + SELECT name FROM fts3_shadow_tables + ) + SELECT name FROM final ORDER BY 1 + """ - select name from sqlite_master - where rootpage = 0 - and ( - sql like '%VIRTUAL TABLE%USING FTS%' - ) or name in ('sqlite_stat1', 'sqlite_stat2', 'sqlite_stat3', 'sqlite_stat4') - or name like '\\_%' escape '\\' - """ ) - ).rows - ] + ] + has_spatialite = await self.execute_fn(detect_spatialite) if has_spatialite: # Also hide Spatialite internal tables @@ -503,19 +577,6 @@ async def hidden_table_names(self): ) ).rows ] - # Add any tables marked as hidden in config - db_config = self.ds.config.get("databases", {}).get(self.name, {}) - if "tables" in db_config: - hidden_tables += [ - t for t in db_config["tables"] if db_config["tables"][t].get("hidden") - ] - # Also mark as hidden any tables which start with the name of a hidden table - # e.g. "searchable_fts" implies "searchable_fts_content" should be hidden - for table_name in await self.table_names(): - for hidden_table in hidden_tables[:]: - if table_name.startswith(hidden_table): - hidden_tables.append(table_name) - continue return hidden_tables diff --git a/tests/test_api.py b/tests/test_api.py index 431ab5cee4..01c9bb79ad 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -389,6 +389,29 @@ async def test_database_page(ds_client): }, "private": False, }, + { + "name": "searchable_fts", + "columns": [ + "text1", + "text2", + "name with . and spaces", + ] + + ( + [ + "searchable_fts", + "docid", + "__langid", + ] + if supports_table_xinfo() + else [] + ), + "primary_keys": [], + "count": 2, + "hidden": False, + "fts_table": "searchable_fts", + "foreign_keys": {"incoming": [], "outgoing": []}, + "private": False, + }, { "name": "searchable_tags", "columns": ["searchable_id", "tag"], @@ -525,29 +548,6 @@ async def test_database_page(ds_client): "foreign_keys": {"incoming": [], "outgoing": []}, "private": False, }, - { - "name": "searchable_fts", - "columns": [ - "text1", - "text2", - "name with . and spaces", - ] - + ( - [ - "searchable_fts", - "docid", - "__langid", - ] - if supports_table_xinfo() - else [] - ), - "primary_keys": [], - "count": 2, - "hidden": True, - "fts_table": "searchable_fts", - "foreign_keys": {"incoming": [], "outgoing": []}, - "private": False, - }, { "name": "searchable_fts_docsize", "columns": ["docid", "size"], diff --git a/tests/test_html.py b/tests/test_html.py index ae27048633..4d95a8fa07 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -41,13 +41,14 @@ def test_homepage(app_client_two_attached_databases): assert "extra database" == h2.text.strip() counts_p, links_p = h2.find_all_next("p")[:2] assert ( - "2 rows in 1 table, 5 rows in 4 hidden tables, 1 view" == counts_p.text.strip() + "4 rows in 2 tables, 3 rows in 3 hidden tables, 1 view" == counts_p.text.strip() ) # We should only show visible, not hidden tables here: table_links = [ {"href": a["href"], "text": a.text.strip()} for a in links_p.findAll("a") ] assert [ + {"href": r"/extra+database/searchable_fts", "text": "searchable_fts"}, {"href": r"/extra+database/searchable", "text": "searchable"}, {"href": r"/extra+database/searchable_view", "text": "searchable_view"}, ] == table_links diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index 1c155cf3b3..bc3c8fcfad 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -4,7 +4,7 @@ from datasette.app import Datasette from datasette.database import Database, Results, MultipleValues -from datasette.utils.sqlite import sqlite3 +from datasette.utils.sqlite import sqlite3, sqlite_version from datasette.utils import Column from .fixtures import app_client, app_client_two_attached_databases_crossdb_enabled import pytest @@ -664,3 +664,50 @@ async def test_in_memory_databases_forbid_writes(app_client): # Using db.execute_write() should work: await db.execute_write("create table foo (t text)") assert await db.table_names() == ["foo"] + + +def pragma_table_list_supported(): + return sqlite_version()[1] >= 37 + + +@pytest.mark.asyncio +@pytest.mark.skipif( + not pragma_table_list_supported(), reason="Requires PRAGMA table_list support" +) +async def test_hidden_tables(app_client): + ds = app_client.ds + db = ds.add_database(Database(ds, is_memory=True, is_mutable=True)) + assert await db.hidden_table_names() == [] + await db.execute("create virtual table f using fts5(a)") + assert await db.hidden_table_names() == [ + "f_config", + "f_content", + "f_data", + "f_docsize", + "f_idx", + ] + + await db.execute("create virtual table r using rtree(id, amin, amax)") + assert await db.hidden_table_names() == [ + "f_config", + "f_content", + "f_data", + "f_docsize", + "f_idx", + "r_node", + "r_parent", + "r_rowid", + ] + + await db.execute("create table _hideme(_)") + assert await db.hidden_table_names() == [ + "_hideme", + "f_config", + "f_content", + "f_data", + "f_docsize", + "f_idx", + "r_node", + "r_parent", + "r_rowid", + ]