mirror of
https://github.com/simonw/datasette.git
synced 2026-06-14 04:56:59 +02:00
Switch to CTE to handle 600+ actions at once
GPT-5.5 xhigh in Codex spotted this problem and fixed it with a CTE: https://gisthost.github.io/?46076499ee685acddc988ff6b47a74b0
This commit is contained in:
parent
bb59c61c9f
commit
86334d233d
2 changed files with 49 additions and 4 deletions
|
|
@ -555,7 +555,7 @@ async def check_permissions_for_actions(
|
|||
|
||||
params = {"_check_parent": parent, "_check_child": child}
|
||||
ctes = []
|
||||
selects = []
|
||||
result_rows = []
|
||||
verdicts = {}
|
||||
|
||||
for i, (action, permission_sqls) in enumerate(zip(unique_actions, gathered)):
|
||||
|
|
@ -627,10 +627,17 @@ async def check_permissions_for_actions(
|
|||
AND (r.child = :_check_child OR r.child IS NULL)
|
||||
)"""
|
||||
|
||||
selects.append(f"SELECT {i} AS action_idx, ({verdict_sql}) AS is_allowed")
|
||||
result_rows.append(f"({i}, ({verdict_sql}))")
|
||||
|
||||
if selects:
|
||||
query = "WITH\n" + ",\n".join(ctes) + "\n" + "\nUNION ALL\n".join(selects)
|
||||
if result_rows:
|
||||
ctes.append(
|
||||
"results(action_idx, is_allowed) AS (VALUES\n"
|
||||
+ ",\n".join(result_rows)
|
||||
+ "\n)"
|
||||
)
|
||||
query = (
|
||||
"WITH\n" + ",\n".join(ctes) + "\nSELECT action_idx, is_allowed FROM results"
|
||||
)
|
||||
result = await datasette.get_internal_database().execute(query, params)
|
||||
for row in result.rows:
|
||||
verdicts[unique_actions[row[0]]] = bool(row[1])
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import pytest
|
|||
import pytest_asyncio
|
||||
from datasette.app import Datasette
|
||||
from datasette.permissions import (
|
||||
Action,
|
||||
PermissionSQL,
|
||||
SkipPermissions,
|
||||
_permission_check_cache,
|
||||
|
|
@ -541,6 +542,43 @@ async def test_allowed_many_skip_permission_checks(ds):
|
|||
assert results == {"view-table": True, "drop-table": True}
|
||||
|
||||
|
||||
class ManyActionsPlugin:
|
||||
"""Registers enough actions to exceed SQLite's compound SELECT limit."""
|
||||
|
||||
def __init__(self, count):
|
||||
self.action_names = [f"bulk-action-{i}" for i in range(count)]
|
||||
self.action_names_set = set(self.action_names)
|
||||
|
||||
@hookimpl
|
||||
def register_actions(self, datasette):
|
||||
return [
|
||||
Action(name=name, abbr=None, description="Bulk test action")
|
||||
for name in self.action_names
|
||||
]
|
||||
|
||||
@hookimpl
|
||||
def permission_resources_sql(self, datasette, actor, action):
|
||||
if action in self.action_names_set:
|
||||
return PermissionSQL(
|
||||
sql="SELECT NULL AS parent, NULL AS child, 1 AS allow, 'bulk allow' AS reason",
|
||||
params={},
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_allowed_many_more_than_sqlite_compound_select_limit():
|
||||
plugin = ManyActionsPlugin(600)
|
||||
ds = Datasette()
|
||||
ds.pm.register(plugin, name="many_actions")
|
||||
try:
|
||||
await ds.invoke_startup()
|
||||
results = await ds.allowed_many(actions=plugin.action_names, actor=None)
|
||||
assert len(results) == 600
|
||||
assert all(results.values())
|
||||
finally:
|
||||
ds.pm.unregister(name="many_actions")
|
||||
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# Layer 3: precompute before table_actions / database_actions hooks
|
||||
# ----------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue