Add extended stats and force of custom query plans

CHANGELOG.md

@@ -35,11 +35,12 @@ Write the date in place of the "Unreleased" in the case a new version is release
   node associations but lose their grants, preventing transient configuration
   failures from erasing access-control information. The server remains
   compatible with older clients that use `access_blob`.
-- Deployment note: this change includes three sequential catalog migrations
-  (an intermediate blob-association schema, conversion to tags, and a
-  parent-scoped node-tag association). Apply the full migration chain without
-  stopping at an intermediate revision. This also drops the old metadata
-  index; deploy the improved replacement index from #1521 as well.
+- Deployment note: this change includes four sequential catalog migrations
+  (an intermediate blob-association schema, conversion to tags, a
+  parent-scoped node-tag association, and naming adjustment). Apply the full
+  migration chain without stopping at an intermediate revision. This also drops
+  the old metadata index; deploy the improved replacement index from #1521
+  as well.
 - Object-storage (S3) tests now run against an in-process `moto` S3 server
   started automatically by the test suite, instead of requiring a MinIO
   container (whose image was removed from Docker Hub). Set `TILED_TEST_BUCKET`

tests/test_access_control.py

@@ -536,15 +536,15 @@ def _principal_has_scope_on_access_tag(
     """
     sql = (
         "SELECT 1 "
-        "FROM access_tag_principal_scopes_association aps "
-        "JOIN access_tags t ON t.id = aps.tag_id "
-        "JOIN access_tags_principals p ON p.id = aps.principal_id "
+        "FROM access_grants ag "
+        "JOIN access_tags t ON t.id = ag.tag_id "
+        "JOIN access_tags_principals p ON p.id = ag.principal_id "
     )
     params = {"t": access_tag_name, "p": principal}
     sql += "WHERE t.name = :t AND p.name = :p"
     if scope_name is not None:
         params["s"] = scope_name
-        sql += " AND aps.scope = :s"
+        sql += " AND ag.scope = :s"
     rows = catalog_db_execute(catalog_uri, [(sql, params)])
     return bool(rows)
 
@@ -707,7 +707,7 @@ async def test_catalog_rejects_an_invalid_scope(
             async with engine.begin() as conn:
                 await conn.execute(
                     text(
-                        "INSERT INTO access_tag_principal_scopes_association "
+                        "INSERT INTO access_grants "
                         "(tag_id, principal_id, scope) "
                         "VALUES (:t, :p, 'not:a:real:scope')"
                     ),
@@ -927,10 +927,10 @@ async def _tags_granting_sue_delete():
             result = await connection.execute(
                 text(
                     "SELECT t.name "
-                    "FROM access_tag_principal_scopes_association aps "
-                    "JOIN access_tags t ON t.id = aps.tag_id "
-                    "JOIN access_tags_principals p ON p.id = aps.principal_id "
-                    "WHERE p.name = :p AND aps.scope = :s"
+                    "FROM access_grants ag "
+                    "JOIN access_tags t ON t.id = ag.tag_id "
+                    "JOIN access_tags_principals p ON p.id = ag.principal_id "
+                    "WHERE p.name = :p AND ag.scope = :s"
                 ),
                 {"p": "sue", "s": "delete:node"},
             )

tiled/access_control/access_tags.py

@@ -19,21 +19,18 @@
 # Name of the scope enum, shared by the PostgreSQL enum type and the SQLite
 # CHECK constraint that stands in for it. Taken from the ORM so that the two
 # cannot drift.
-SCOPE_ENUM_NAME = orm.AccessTagPrincipalScopeAssociation.__table__.c.scope.type.name
+SCOPE_ENUM_NAME = orm.AccessGrant.__table__.c.scope.type.name
 
-# The access_tag_principal_scopes_association junction table, joined to the tables its
-# two foreign keys reference so that it can be queried by name. The scope is
-# stored inline on the junction and needs no join. Shared by the lookups in
+# The access_grants junction table, joined to the tables its two foreign keys
+# reference so that it can be queried by name. The scope is stored inline on
+# the junction and needs no join. Shared by the lookups in
 # both directions: (tag, principal) -> scopes and (principal, scope) -> tags.
-access_tag_principal_scopes_association_named = (
-    orm.AccessTagPrincipalScopeAssociation.__table__.join(
-        orm.AccessTag.__table__,
-        orm.AccessTag.id == orm.AccessTagPrincipalScopeAssociation.tag_id,
-    ).join(
-        orm.AccessTagsPrincipal.__table__,
-        orm.AccessTagsPrincipal.id
-        == orm.AccessTagPrincipalScopeAssociation.principal_id,
-    )
+access_grants_named = orm.AccessGrant.__table__.join(
+    orm.AccessTag.__table__,
+    orm.AccessTag.id == orm.AccessGrant.tag_id,
+).join(
+    orm.AccessTagsPrincipal.__table__,
+    orm.AccessTagsPrincipal.id == orm.AccessGrant.principal_id,
 )
 
 
@@ -83,7 +80,7 @@ async def get_enforced_scopes(self):
             else:
                 constraints = await conn.run_sync(
                     lambda sync: inspect(sync).get_check_constraints(
-                        orm.AccessTagPrincipalScopeAssociation.__tablename__
+                        orm.AccessGrant.__tablename__
                     )
                 )
                 for constraint in constraints:
@@ -100,8 +97,8 @@ async def get_enforced_scopes(self):
 
     async def get_scopes_from_tag(self, tagname, username):
         statement = (
-            select(orm.AccessTagPrincipalScopeAssociation.scope)
-            .select_from(access_tag_principal_scopes_association_named)
+            select(orm.AccessGrant.scope)
+            .select_from(access_grants_named)
             .where(
                 orm.AccessTag.name == tagname,
                 orm.AccessTagsPrincipal.name == username,
@@ -142,9 +139,9 @@ async def is_tag_public(self, name):
     async def get_tags_from_scope(self, scope, username):
         statement = (
             select(orm.AccessTag.name)
-            .select_from(access_tag_principal_scopes_association_named)
+            .select_from(access_grants_named)
             .where(
-                orm.AccessTagPrincipalScopeAssociation.scope == scope,
+                orm.AccessGrant.scope == scope,
                 orm.AccessTagsPrincipal.name == username,
             )
         )
@@ -162,7 +159,7 @@ async def get_tags_from_scope(self, scope, username):
 ACCESS_TAGS_TABLES = [
     orm.AccessTag.__table__,
     orm.AccessTagsPrincipal.__table__,
-    orm.AccessTagPrincipalScopeAssociation.__table__,
+    orm.AccessGrant.__table__,
     orm.AccessTagOwnerAssociation.__table__,
 ]
 
@@ -222,7 +219,7 @@ async def update_access_tags_tables(engine, tags, owners, public_tags):
     upsert = _upsert(engine)
     tags_table = orm.AccessTag.__table__
     users_table = orm.AccessTagsPrincipal.__table__
-    tags_users_scopes_table = orm.AccessTagPrincipalScopeAssociation.__table__
+    access_grants_table = orm.AccessGrant.__table__
     tag_owners_table = orm.AccessTagOwnerAssociation.__table__
 
     # stage all items in memory, deduplicated
@@ -358,9 +355,9 @@ async def update_access_tags_tables(engine, tags, owners, public_tags):
             tuple(row)
             for row in await connection.execute(
                 select(
-                    tags_users_scopes_table.c.tag_id,
-                    tags_users_scopes_table.c.principal_id,
-                    tags_users_scopes_table.c.scope,
+                    access_grants_table.c.tag_id,
+                    access_grants_table.c.principal_id,
+                    access_grants_table.c.scope,
                 )
             )
         }
@@ -375,7 +372,7 @@ async def update_access_tags_tables(engine, tags, owners, public_tags):
         new_tags_users_scopes = tags_users_scopes - existing_tags_users_scopes
         if new_tags_users_scopes:
             await connection.execute(
-                insert(tags_users_scopes_table),
+                insert(access_grants_table),
                 [
                     {"tag_id": tag_id, "principal_id": user_id, "scope": scope}
                     for tag_id, user_id, scope in new_tags_users_scopes
@@ -384,11 +381,11 @@ async def update_access_tags_tables(engine, tags, owners, public_tags):
         stale_tags_users_scopes = existing_tags_users_scopes - tags_users_scopes
         if stale_tags_users_scopes:
             await connection.execute(
-                delete(tags_users_scopes_table).where(
+                delete(access_grants_table).where(
                     tuple_(
-                        tags_users_scopes_table.c.tag_id,
-                        tags_users_scopes_table.c.principal_id,
-                        tags_users_scopes_table.c.scope,
+                        access_grants_table.c.tag_id,
+                        access_grants_table.c.principal_id,
+                        access_grants_table.c.scope,
                     ).in_(list(stale_tags_users_scopes))
                 )
             )

tiled/catalog/adapter.py

@@ -129,6 +129,7 @@
     check_catalog_database,
     initialize_database,
     register_principal_tag_rows,
+    resolve_access_tag_ids,
 )
 from .explain import ExplainAsyncSession
 from .utils import compute_structure_id
@@ -559,13 +560,8 @@ async def resolve_access_tag_ids(self, names):
         filters that reference 'tag_id' literals instead of joining
         'access_tags' by name -- see 'access_tags_filter'.
         """
-        if not names:
-            return []
         async with self.context.session() as db:
-            result = await db.execute(
-                select(orm.AccessTag.id).where(orm.AccessTag.name.in_(list(names)))
-            )
-            return list(result.scalars().all())
+            return await resolve_access_tag_ids(db, names)
 
     async def exact_len(self):
         "Get the exact number of child nodes."

tiled/catalog/core.py

@@ -6,6 +6,7 @@
 
 # This is list of all valid revisions (from current to oldest).
 ALL_REVISIONS = [
+    "917dbbcfc4b1",
     "3e87cbeb195b",
     "0d4e1f2a3b4c",
     "3bc110ce44e9",
@@ -38,6 +39,7 @@
 
 
 async def initialize_database(engine: AsyncEngine):
+    # The definitions in .orm alter Base.metadata.
     from ..graph import orm as graph_orm  # noqa: F401
     from . import orm  # noqa: F401
 
@@ -99,13 +101,14 @@ async def register_principal_tag_rows(connection, access_tag_names):
     `await session.connection()`.)
     """
     from ..access_control.protocols import PRINCIPAL_TAG_PREFIXES
-    from . import orm
 
     principal_tags = {
         name for name in access_tag_names if name.startswith(PRINCIPAL_TAG_PREFIXES)
     }
     if not principal_tags:
         return
+    from . import orm
+
     if connection.dialect.name == "postgresql":
         from sqlalchemy.dialects.postgresql import insert as upsert
     else:
@@ -117,6 +120,18 @@ async def register_principal_tag_rows(connection, access_tag_names):
     )
 
 
+async def resolve_access_tag_ids(connection, names):
+    """Resolve existing access-tag names to their integer IDs."""
+    if not names:
+        return []
+    from . import orm
+
+    result = await connection.execute(
+        select(orm.AccessTag.id).where(orm.AccessTag.name.in_(list(names)))
+    )
+    return list(result.scalars().all())
+
+
 async def check_catalog_database(engine: AsyncEngine):
     redacted_url = engine.url._replace(password="[redacted]")
     try:

tiled/catalog/migrations/versions/0d4e1f2a3b4c_add_parent_to_node_access_tags.py

@@ -19,6 +19,7 @@
 depends_on = None
 
 INDEX_NAME = "ix_node_access_tags_association_parent_id_tag_id_node_id"
+STATISTICS_NAME = "node_access_tags_association_parent_id_tag_id_node_id_stats"
 
 # Tune these for the database host running the migration. These settings apply
 # only to this migration's PostgreSQL connection. work_mem may be consumed by
@@ -165,7 +166,15 @@ def upgrade():
             "node_access_tags_association",
             ["parent_id", "tag_id", "node_id"],
         )
-        op.execute("ANALYZE node_access_tags_association")
+        op.execute(
+            f"CREATE STATISTICS IF NOT EXISTS {STATISTICS_NAME} "
+            "(dependencies, ndistinct, mcv) "
+            "ON parent_id, tag_id FROM node_access_tags_association"
+        )
+        # VACUUM cannot run inside a transaction. Creating the statistics before
+        # this also ensures that VACUUM ANALYZE populates them immediately.
+        with op.get_context().autocommit_block():
+            connection.execute(sa.text("VACUUM ANALYZE node_access_tags_association"))
     else:
         op.execute(
             """
@@ -189,6 +198,7 @@ def downgrade():
 
     _drop_triggers(connection)
     if dialect_name == "postgresql":
+        op.execute(f"DROP STATISTICS IF EXISTS {STATISTICS_NAME}")
         op.drop_index(
             INDEX_NAME,
             table_name="node_access_tags_association",