Skip to content

Replace group hierarchy BFS N+1 queries with recursive CTEs #1443

Description

@ptone

Summary

The group hierarchy traversals in pkg/store/entadapter/group_store.go use application-level BFS with one query per tree level (N+1 pattern). These can be replaced with single WITH RECURSIVE CTE queries, eliminating per-level round trips.

Current state

Two BFS functions in group_store.go:745-828:

hasPathDown() (~line 745)

Walks child_groups edge downward to check if a path exists between two groups. Max depth 10. Each BFS step issues:

s.client.Group.Query().Where(...).QueryChildGroups().All(ctx)

One query per level of the tree.

GetEffectiveGroups() (~line 800)

BFS upward through parent_groups to collect all ancestor groups for a user's memberships. Same N+1 pattern — one query per level.

Both are bounded at 10 levels, so the worst case is 10 sequential queries per call. In practice, group hierarchies are typically 2-4 levels deep.

Proposed fix

Replace each BFS with a single WITH RECURSIVE CTE:

WITH RECURSIVE ancestors AS (
    SELECT parent_group_id FROM group_children WHERE child_group_id = $1
    UNION
    SELECT gc.parent_group_id FROM group_children gc
    JOIN ancestors a ON gc.child_group_id = a.parent_group_id
)
SELECT id FROM groups WHERE id IN (SELECT parent_group_id FROM ancestors);

This is standard SQL supported by both PostgreSQL and SQLite (SQLite has supported WITH RECURSIVE since 3.8.3 / 2014), so dual-dialect compatibility is maintained.

Why this instead of PG19 SQL/PGQ

A strategic assessment of PG19's new SQL/PGQ graph query support concluded that Scion's graph traversals are too shallow (mostly 1-2 hops) and PG19's graph indexing too immature to justify adoption. Recursive CTEs solve the only concrete N+1 performance issue today, with no version dependency.

Assessment: scratchpad/projects/roadmap/inv-pg19-graph-ent-assessment.md

Files

  • pkg/store/entadapter/group_store.go:745-828 — the BFS functions to replace
  • pkg/hub/authz_delegation_ceiling.go — delegation chain walk (similar pattern but operates on flat table, lower priority)
  • pkg/store/entadapter/dialect.go — existing dialect-specific SQL patterns (reference for CTE integration)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:hubArea: hubtype:choreMaintenance / infra / non-feature work

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions