perf: GetSessions runs 11 correlated subqueries per row despite its doc comment #453

New issue

Open

opened 2026-07-25 23:32:22 +02:00 by dries · 0 comments

dries commented

2026-07-25 23:32:22 +02:00

Owner

Code-quality audit (P2). internal/db/sessions.go:22-101.

// GetSessions returns sessions, optionally filtered by directory and/or a minimum timestamp.
// Uses SQL aggregation to avoid N+1 queries.
func (d *DB) GetSessions(directory string, since int64) ([]Session, error) {

The doc comment is wrong. The query runs 11 correlated subqueries per session row:

1 count(*) over message
3 SUM(...) scans over message
7 independent ... ORDER BY time_created DESC LIMIT 1 subqueries that each re-find the same last message: last_role, last_finish, last_error, last_error_name, last_error_message, last_error_at, last_synth_terminal.

The N+1 was moved from Go into SQL, not removed. There is also no LIMIT on the statement, so an unfiltered call does all of this for every session in the table.

Why it matters

This is on the /api/sessions request path — the dashboard's first paint and every poll. Cost grows with total session count, not page size, and each of the 7 last-message subqueries re-scans the same message rows. Combined with the missing ExecContext/QueryContext (separate ticket), a slow run cannot even be cancelled.

Suggested fix

Collapse the 7 into one LEFT JOIN against a last_message CTE keyed on session_id:

WITH last_message AS (
  SELECT session_id, data, time_created,
         ROW_NUMBER() OVER (PARTITION BY session_id ORDER BY time_created DESC) AS rn
  FROM message
)
... FROM session s LEFT JOIN last_message lm ON lm.session_id = s.id AND lm.rn = 1

Same treatment for the 3 SUMs + count(*) via a single grouped aggregate CTE. Fix the doc comment either way. Consider a LIMIT on the unfiltered path.

Acceptance criteria

One pass over message per aggregate concern instead of 11 correlated subqueries.
Identical results: existing GetSessions tests pass unchanged, including the last_synth_terminal shell-envelope case.
Doc comment describes what the query actually does.

Effort: M.

Code-quality audit (P2). `internal/db/sessions.go:22-101`. ```go // GetSessions returns sessions, optionally filtered by directory and/or a minimum timestamp. // Uses SQL aggregation to avoid N+1 queries. func (d *DB) GetSessions(directory string, since int64) ([]Session, error) { ``` The doc comment is wrong. The query runs **11 correlated subqueries per session row**: - 1 `count(*)` over `message` - 3 `SUM(...)` scans over `message` - **7** independent `... ORDER BY time_created DESC LIMIT 1` subqueries that each re-find the *same* last message: `last_role`, `last_finish`, `last_error`, `last_error_name`, `last_error_message`, `last_error_at`, `last_synth_terminal`. The N+1 was moved from Go into SQL, not removed. There is also **no `LIMIT`** on the statement, so an unfiltered call does all of this for every session in the table. ## Why it matters This is on the `/api/sessions` request path — the dashboard's first paint and every poll. Cost grows with total session count, not page size, and each of the 7 last-message subqueries re-scans the same `message` rows. Combined with the missing `ExecContext`/`QueryContext` (separate ticket), a slow run cannot even be cancelled. ## Suggested fix Collapse the 7 into one `LEFT JOIN` against a `last_message` CTE keyed on `session_id`: ```sql WITH last_message AS ( SELECT session_id, data, time_created, ROW_NUMBER() OVER (PARTITION BY session_id ORDER BY time_created DESC) AS rn FROM message ) ... FROM session s LEFT JOIN last_message lm ON lm.session_id = s.id AND lm.rn = 1 ``` Same treatment for the 3 `SUM`s + `count(*)` via a single grouped aggregate CTE. Fix the doc comment either way. Consider a `LIMIT` on the unfiltered path. ## Acceptance criteria - [ ] One pass over `message` per aggregate concern instead of 11 correlated subqueries. - [ ] Identical results: existing `GetSessions` tests pass unchanged, including the `last_synth_terminal` shell-envelope case. - [ ] Doc comment describes what the query actually does. Effort: M.