From 5138e95d694e816622a4cb12b7fb3d243d74fa05 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 24 Oct 2025 09:58:58 -0700 Subject: [PATCH] Migrate homepage to use bulk allowed_resources() and fix NULL handling in SQL JOINs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Updated IndexView in datasette/views/index.py to fetch all allowed databases and tables in bulk upfront using allowed_resources() instead of calling check_visibility() for each database, table, and view individually - Fixed SQL bug in build_allowed_resources_sql() where USING (parent, child) clauses failed for database resources because NULL = NULL evaluates to NULL in SQL, not TRUE - Changed all INNER JOINs to use explicit ON conditions with NULL-safe comparisons: ON b.parent = x.parent AND (b.child = x.child OR (b.child IS NULL AND x.child IS NULL)) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- datasette/utils/actions_sql.py | 16 ++++---- datasette/views/index.py | 67 +++++++++++++++++++++------------- 2 files changed, 51 insertions(+), 32 deletions(-) diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index 811e782c..4088e988 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -255,9 +255,9 @@ async def build_allowed_resources_sql( " ELSE 0", " END AS anon_is_allowed", " FROM base b", - " JOIN anon_child_lvl acl USING (parent, child)", - " JOIN anon_parent_lvl apl USING (parent, child)", - " JOIN anon_global_lvl agl USING (parent, child)", + " JOIN anon_child_lvl acl ON b.parent = acl.parent AND (b.child = acl.child OR (b.child IS NULL AND acl.child IS NULL))", + " JOIN anon_parent_lvl apl ON b.parent = apl.parent AND (b.child = apl.child OR (b.child IS NULL AND apl.child IS NULL))", + " JOIN anon_global_lvl agl ON b.parent = agl.parent AND (b.child = agl.child OR (b.child IS NULL AND agl.child IS NULL))", "),", ] ) @@ -306,14 +306,16 @@ async def build_allowed_resources_sql( query_parts.extend( [ " FROM base b", - " JOIN child_lvl cl USING (parent, child)", - " JOIN parent_lvl pl USING (parent, child)", - " JOIN global_lvl gl USING (parent, child)", + " JOIN child_lvl cl ON b.parent = cl.parent AND (b.child = cl.child OR (b.child IS NULL AND cl.child IS NULL))", + " JOIN parent_lvl pl ON b.parent = pl.parent AND (b.child = pl.child OR (b.child IS NULL AND pl.child IS NULL))", + " JOIN global_lvl gl ON b.parent = gl.parent AND (b.child = gl.child OR (b.child IS NULL AND gl.child IS NULL))", ] ) if include_is_private: - query_parts.append(" JOIN anon_decisions ad USING (parent, child)") + query_parts.append( + " JOIN anon_decisions ad ON b.parent = ad.parent AND (b.child = ad.child OR (b.child IS NULL AND ad.child IS NULL))" + ) query_parts.append(")") diff --git a/datasette/views/index.py b/datasette/views/index.py index 63cc067d..2939f98e 100644 --- a/datasette/views/index.py +++ b/datasette/views/index.py @@ -26,27 +26,47 @@ class IndexView(BaseView): async def get(self, request): as_format = request.url_vars["format"] await self.ds.ensure_permissions(request.actor, ["view-instance"]) + + # Get all allowed databases and tables in bulk + allowed_databases = await self.ds.allowed_resources( + "view-database", request.actor, include_is_private=True + ) + allowed_db_dict = {r.parent: r for r in allowed_databases} + + allowed_tables = await self.ds.allowed_resources( + "view-table", request.actor, include_is_private=True + ) + # Group by database + tables_by_db = {} + for t in allowed_tables: + if t.parent not in tables_by_db: + tables_by_db[t.parent] = {} + tables_by_db[t.parent][t.child] = t + databases = [] - for name, db in self.ds.databases.items(): - database_visible, database_private = await self.ds.check_visibility( - request.actor, - "view-database", - name, - ) - if not database_visible: - continue - table_names = await db.table_names() + # Iterate over allowed databases instead of all databases + for name in allowed_db_dict.keys(): + db = self.ds.databases[name] + database_private = allowed_db_dict[name].private + + # Get allowed tables/views for this database + allowed_for_db = tables_by_db.get(name, {}) + + # Get table names from allowed set instead of db.table_names() + table_names = [child_name for child_name in allowed_for_db.keys()] + hidden_table_names = set(await db.hidden_table_names()) - views = [] - for view_name in await db.view_names(): - view_visible, view_private = await self.ds.check_visibility( - request.actor, - "view-table", - (name, view_name), - ) - if view_visible: - views.append({"name": view_name, "private": view_private}) + # Determine which allowed items are views + view_names_set = set(await db.view_names()) + views = [ + {"name": child_name, "private": resource.private} + for child_name, resource in allowed_for_db.items() + if child_name in view_names_set + ] + + # Filter to just tables (not views) for table processing + table_names = [name for name in table_names if name not in view_names_set] # Perform counts only for immutable or DBS with <= COUNT_TABLE_LIMIT tables table_counts = {} @@ -58,13 +78,10 @@ class IndexView(BaseView): tables = {} for table in table_names: - visible, private = await self.ds.check_visibility( - request.actor, - "view-table", - (name, table), - ) - if not visible: + # Check if table is in allowed set + if table not in allowed_for_db: continue + table_columns = await db.table_columns(table) tables[table] = { "name": table, @@ -74,7 +91,7 @@ class IndexView(BaseView): "hidden": table in hidden_table_names, "fts_table": await db.fts_table(table), "num_relationships_for_sorting": 0, - "private": private, + "private": allowed_for_db[table].private, } if request.args.get("_sort") == "relationships" or not table_counts: