N+1 queries: the bug that survives every code review

Of all the performance bugs I’ve shipped, the N+1 query is the one that keeps coming back, in every codebase, in every language, no matter how careful the team is. It’s not subtle once you know to look for it, and it’s still one of the easiest things to write by accident, because the code that causes it reads completely normally.

The dashboard that worked fine until it didn’t

We had an internal admin page listing every user in an account, with each row showing the user’s name and their team’s name. The handler looked roughly like this:

func listUsers(ctx context.Context, db *sql.DB, accountID string) ([]UserRow, error) {
    users, err := queryUsers(ctx, db, accountID)
    if err != nil {
        return nil, err
    }

    rows := make([]UserRow, 0, len(users))
    for _, u := range users {
        team, err := queryTeamName(ctx, db, u.TeamID) // one query per user
        if err != nil {
            return nil, err
        }
        rows = append(rows, UserRow{User: u, TeamName: team})
    }
    return rows, nil
}

This shipped, passed review, and worked fine in staging, where the test account had about thirty users. It kept working fine in production too, for months, because most of our customers had a few dozen employees. Then a customer with about four thousand users opened that page, and the handler issued one query to fetch the user list plus four thousand more, one per row, sequentially, each one a full network round trip to Postgres. The endpoint took upward of nine seconds, the request-scoped connection got held the entire time, and with a handful of people from that account opening the page around the same time, the connection pool ran out and started queueing every other request in the process behind it, unrelated ones included.

Nothing about that loop is wrong Go, and nothing about it looks wrong on review. It’s a for loop that looks up a related field for each item, which is such an ordinary shape that a reviewer’s eye slides right past it. The cost is entirely invisible in the diff: one line, queryTeamName(ctx, db, u.TeamID), gives no hint that it’s a full round trip to the database, and the loop gives no hint that it runs that line a number of times decided by production data nobody tested against.

The fix: batch the lookup, don’t repeat it

The fix isn’t a different tool, it’s the same query written once instead of once per row:

func listUsers(ctx context.Context, db *sql.DB, accountID string) ([]UserRow, error) {
    users, err := queryUsers(ctx, db, accountID)
    if err != nil {
        return nil, err
    }

    teamIDs := make([]string, 0, len(users))
    seen := make(map[string]bool)
    for _, u := range users {
        if !seen[u.TeamID] {
            teamIDs = append(teamIDs, u.TeamID)
            seen[u.TeamID] = true
        }
    }

    teamNames, err := queryTeamNames(ctx, db, teamIDs) // one query, map[teamID]name
    if err != nil {
        return nil, err
    }

    rows := make([]UserRow, 0, len(users))
    for _, u := range users {
        rows = append(rows, UserRow{User: u, TeamName: teamNames[u.TeamID]})
    }
    return rows, nil
}

queryTeamNames runs a single query with WHERE id = ANY($1) against the deduplicated slice of team IDs and returns a map, so the per-row work in the second loop is a map lookup, not a network call. Four thousand and one queries becomes two, regardless of how many users are on the page. The four-thousand-user account went from nine seconds to under fifty milliseconds.

A join would have worked too, and for this shape, a single SELECT users.*, teams.name FROM users JOIN teams ON teams.id = users.team_id WHERE users.account_id = $1 is arguably the more natural fix. I’ve shown the batch-and-map version because it generalizes better to cases a join doesn’t handle as cleanly, fetching from a different service or table entirely, or combining several unrelated per-row lookups into one pass.

Why this keeps happening anyway

Knowing the fix doesn’t prevent the bug, because the bug isn’t a knowledge gap. Everyone on the team already knew not to query in a loop. It got written anyway because the loop was added to already-working code weeks after the original endpoint shipped, as a small, locally-reasonable change: “also show the team name,” implemented the way that required touching the least code at the time. Nobody sat down to design an N+1 query. Somebody added one field to a response, and the field happened to live in another table.

Static analysis doesn’t reliably catch this either, because there’s nothing syntactically wrong with calling a function inside a loop. The bug only exists at the semantic level, that function happens to hit a database, and analyzers don’t generally know that without being taught your specific ORM or query layer’s calling convention.

The defense that actually works: make query count visible

The lesson I took from this wasn’t “review loops more carefully,” because that’s exactly what already failed. What actually works is turning query count into a number you can see, per request, the same way you’d already track latency or status code.

We added a request-scoped counter around the database driver, every query increments it, and logged the total alongside the normal access log line for that request. Then we set a budget: any request issuing more than twenty queries gets flagged in a dashboard, not blocked, just visible. An ordinary page render doing three or four queries never comes close. A page with an N+1 bug shows up immediately in staging, once the seed data has more than a handful of rows in the relevant table, long before a customer’s real data volume finds it in production.

That one number turned an entire class of bug from something that hides until a specific customer’s data shape triggers it into something a developer sees in their own terminal the first time they load the page against realistic seed data. The fix for N+1 queries isn’t remembering not to write them. It’s making the query count loud enough that forgetting doesn’t matter.