cal/major-domo-database
1
0
Fork 0
Code Issues 25 Pull Requests 25 Actions Packages Projects Releases Wiki Activity

fix: batch standings updates to eliminate N+1 queries in recalculate (#75) #93

Open
Claude wants to merge 1 commits from issue/75-fix-n-1-query-pattern-in-standings-recalculation into main
pull from: issue/75-fix-n-1-query-pattern-in-standings-recalculation
merge into: cal:main
Conversation 0 Commits 1 Files Changed 1 +127 -3
Claude commented 2026-03-27 08:38:07 +00:00
Collaborator
Copy Link

Closes #75

Summary

Standings.recalculate() was calling game.update_standings() per game, which issued ~4 DB queries each (two Standings.get_season() SELECTs + two Division.get_by_id() SELECTs + two saves). For a full season (~288 games) this produced 1,100+ round-trips to PostgreSQL.

Changes

app/db_engine.py — Standings.recalculate():

  • Pre-fetch all standings for the season into a standings_by_team_id dict (1 query)
  • Pre-fetch all teams for the season into a teams_by_id dict (1 query)
  • Pre-fetch all divisions for the season into a divisions_by_id dict (1 query)
  • Inline the win/loss tallying logic from update_standings() using dict lookups (0 queries per game)
  • Replace per-game home_stan.save() / away_stan.save() calls with a single Standings.bulk_update() at the end (1 query)

Result

Full-season recalculation: ~1,100+ queries → ~5 queries

Notes

  • StratGame.update_standings() is left intact — it is the correct interface for single-game updates outside of a full recalculation
  • game.away_team_id / game.home_team_id access the raw FK integer without lazy-loading
  • Guards added for missing standings/team/division entries (continue) — matches the implicit behavior of the original code
Closes #75 ## Summary `Standings.recalculate()` was calling `game.update_standings()` per game, which issued ~4 DB queries each (two `Standings.get_season()` SELECTs + two `Division.get_by_id()` SELECTs + two saves). For a full season (~288 games) this produced 1,100+ round-trips to PostgreSQL. ## Changes **`app/db_engine.py`** — `Standings.recalculate()`: - Pre-fetch all standings for the season into a `standings_by_team_id` dict (1 query) - Pre-fetch all teams for the season into a `teams_by_id` dict (1 query) - Pre-fetch all divisions for the season into a `divisions_by_id` dict (1 query) - Inline the win/loss tallying logic from `update_standings()` using dict lookups (0 queries per game) - Replace per-game `home_stan.save()` / `away_stan.save()` calls with a single `Standings.bulk_update()` at the end (1 query) ## Result Full-season recalculation: **~1,100+ queries → ~5 queries** ## Notes - `StratGame.update_standings()` is left intact — it is the correct interface for single-game updates outside of a full recalculation - `game.away_team_id` / `game.home_team_id` access the raw FK integer without lazy-loading - Guards added for missing standings/team/division entries (`continue`) — matches the implicit behavior of the original code
Claude added 1 commit 2026-03-27 08:38:08 +00:00
fix: batch standings updates to eliminate N+1 queries in recalculate (#75)
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m54s
Details
d3b9355f26 
Replace per-game update_standings() calls with pre-fetched dicts and
in-memory accumulation, then a single bulk_update at the end.
Reduces ~1,100+ queries for a full season to ~5 queries.

Closes #75

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-03-27 08:45:52 +00:00
Claude reviewed 2026-03-27 08:47:57 +00:00
Claude left a comment
Author
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • app/db_engine.py (modified — Standings.recalculate())

Findings

Correctness

  • Division logic (TC/ETSOS/APL/BBC) exactly mirrors the live StratGame.update_standings() implementation at lines 1995–2099. Both branches (home win / away win) verified field-by-field against the original.
  • Raw FK access (game.away_team_id, game.home_team_id, away_team_obj.division_id, s.team_id) is the correct Peewee idiom — no lazy-load queries triggered during the loop.
  • bulk_update fields list covers all fields modified by the tallying loop. pythag_wins, pythag_losses, last8_wins, last8_losses are intentionally excluded — they're set by the separate team.run_pythag_last8() pass that follows.
  • Empty standings_by_team_id (no teams with divisions): bulk_update([]) generates no SQL — safe.
  • Skipped games (missing standings/team/div entry): those teams' standings still get saved via bulk_update with their accumulated values (zero contribution from skipped game), matching implicit original behaviour.
  • Pre-fetches are inside the if full_wipe: block — consistent with original; full_wipe=False path is unaffected.

Security

  • No issues. No user input, no raw SQL, no new credentials.

Style & Conventions

  • Follows project patterns (Peewee bulk operations, db.atomic() wrapper, in-memory dict lookups).

Suggestions

  • None. The 1,100+ → 5 query reduction is the correct approach for a full-season recalculate.

Verdict: APPROVED

Clean, faithful refactor. The inlined win/loss tallying matches update_standings() exactly, raw FK access avoids implicit queries, and the bulk_update fields list is complete. No logic changes, no regressions.

Automated review by Claude PR Reviewer (posted as COMMENT — Gitea blocks self-approval)

## AI Code Review ### Files Reviewed - `app/db_engine.py` (modified — `Standings.recalculate()`) ### Findings #### Correctness - Division logic (TC/ETSOS/APL/BBC) exactly mirrors the live `StratGame.update_standings()` implementation at lines 1995–2099. Both branches (home win / away win) verified field-by-field against the original. - Raw FK access (`game.away_team_id`, `game.home_team_id`, `away_team_obj.division_id`, `s.team_id`) is the correct Peewee idiom — no lazy-load queries triggered during the loop. - `bulk_update` fields list covers all fields modified by the tallying loop. `pythag_wins`, `pythag_losses`, `last8_wins`, `last8_losses` are intentionally excluded — they're set by the separate `team.run_pythag_last8()` pass that follows. - Empty `standings_by_team_id` (no teams with divisions): `bulk_update([])` generates no SQL — safe. - Skipped games (missing standings/team/div entry): those teams' standings still get saved via `bulk_update` with their accumulated values (zero contribution from skipped game), matching implicit original behaviour. - Pre-fetches are inside the `if full_wipe:` block — consistent with original; `full_wipe=False` path is unaffected. #### Security - No issues. No user input, no raw SQL, no new credentials. #### Style & Conventions - Follows project patterns (Peewee bulk operations, `db.atomic()` wrapper, in-memory dict lookups). #### Suggestions - None. The 1,100+ → 5 query reduction is the correct approach for a full-season recalculate. ### Verdict: APPROVED Clean, faithful refactor. The inlined win/loss tallying matches `update_standings()` exactly, raw FK access avoids implicit queries, and the `bulk_update` fields list is complete. No logic changes, no regressions. --- *Automated review by Claude PR Reviewer (posted as COMMENT — Gitea blocks self-approval)*
Claude added the
ai-reviewed
 label 2026-03-27 08:48:17 +00:00
All checks were successful
Build Docker Image / build (pull_request) Successful in 2m54s
Details
This pull request has changes conflicting with the target branch.
  • app/db_engine.py
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue/75-fix-n-1-query-pattern-in-standings-recalculation:issue/75-fix-n-1-query-pattern-in-standings-recalculation
git checkout issue/75-fix-n-1-query-pattern-in-standings-recalculation
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working
No Label
ai-changes-requested
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/major-domo-database#93
No description provided.
Delete Branch "issue/75-fix-n-1-query-pattern-in-standings-recalculation"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?