refactor(storage): render joined UPDATEs through the dialect

[Kiran01bm]

Summary

Moves the joined (multi-table) UPDATE statements in the apply comment and control request stores behind the storage Dialect seam. MySQL's rendering is pinned byte-identical; a future PostgreSQL dialect renders the same structured parts as UPDATE ... SET ... FROM ... WHERE. No behavior change for MySQL.

Why

The storage core is dialect-parameterized so a PostgreSQL backend can run the same store code. Multi-table UPDATE syntax is one of the few places MySQL and PostgreSQL diverge at the statement-shape level rather than the fragment level, so the seam hands the whole statement rendering to the dialect.

What

• New Dialect.JoinedUpdate(targetTable, targetAlias, joinTable, joinAlias, joinCondition, assignments, predicate) renders a full joined UPDATE. SET assignments are structured (JoinedUpdateAssignment{Column, Expr}) with unqualified target columns, and the join condition is passed separately from the residual predicate — the two constraints that make PostgreSQL's UPDATE ... FROM shape expressible from the same inputs. The join condition must be placeholder-free (its render position differs per dialect); arguments bind assignments-first, then predicate.
• Five lease-guarded joined UPDATEs converted: apply comments (IncrementEditCount, Supersede, ClearPendingFreeze) and control requests (CompletePending, FailPending).
• Unit tests pin the MySQL renderings exactly, including a placeholder SET expression and the empty-assignments panic.

Joined UPDATE/DELETEs in other store files (applies, operations, tasks) are deliberately out of scope — they follow in the next slices, and a joined-DELETE seam is added there with its first caller.

Before / after

Before                                     After
┌───────────────────────────────┐          ┌───────────────────────────────────┐
│ store hardcodes:              │          │ store passes structured parts:    │
│  UPDATE apply_comments c      │          │  target, join, ON cond,           │
│   JOIN applies a ON ...       │  ──────▶ │  SET assignments, WHERE pred      │
│   SET c.x = ? WHERE ...       │          │ dialect renders the statement:    │
│  (MySQL-only statement shape) │          │  MySQL: UPDATE ... JOIN ... SET   │
└───────────────────────────────┘          │  PG (later): UPDATE ... SET ...   │
                                           │              FROM ... WHERE       │
                                           └───────────────────────────────────┘

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[morgo]

🤖 Approving on Morgan's behalf (automated review, escalation rules apply).

All five joined-UPDATE conversions (IncrementEditCount, Supersede, ClearPendingFreeze, CompletePending, FailPending) render token-identical MySQL to the removed hand-written SQL — same ON condition, SET order, WHERE guards including the lease-token predicate — with argument order and placeholder counts (3/3/3/5/6) verified at every call site, so the RowsAffected-based lost-lease detection is unaffected. Dialect wiring is nil-safe (everything funnels through NewWithDependencies).

One non-blocking hardening suggestion: JoinedUpdate documents that joinCondition must not contain bind placeholders, but nothing enforces it — a future caller's stray ? would silently shift every subsequent binding by one, which in these queries means the lease token binds into the wrong slot. A strings.Contains(joinCondition, "?") panic, symmetric with the existing empty-assignments panic, would pin the contract.

Heads-up: once #1010's Postgres rendering exists, these lease-guarded joined UPDATEs render as UPDATE … FROM, which inherits the fencing question flagged on #1011 — one decision for the whole stack, nothing to change in this PR.

[Copilot]

Pull request overview

Refactors sqlstore’s multi-table (joined) UPDATE statement rendering to go through the Dialect interface, keeping MySQL behavior byte-identical while enabling a future PostgreSQL dialect to render the same structured update as UPDATE ... SET ... FROM ... WHERE.

Changes:

• Adds Dialect.JoinedUpdate(...) plus JoinedUpdateAssignment and implements MySQL rendering in MySQLDialect.
• Converts lease-guarded joined UPDATEs in apply-comment and control-request stores to call the dialect seam.
• Adds unit tests that pin the MySQL joined-UPDATE SQL strings (including placeholder ordering and empty-assignments panic).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/storage/internal/sqlstore/storage.go	Wires Dialect into controlRequestStore so it can render joined UPDATEs via the seam.
pkg/storage/internal/sqlstore/dialect.go	Extends Dialect with JoinedUpdate and adds the MySQL implementation + assignment type.
pkg/storage/internal/sqlstore/dialect_test.go	Pins MySQL joined-UPDATE rendering and validates the empty-assignments panic behavior.
pkg/storage/internal/sqlstore/control_requests.go	Renders lease-guarded joined UPDATEs (complete/fail pending) through Dialect.JoinedUpdate.
pkg/storage/internal/sqlstore/apply_comments.go	Renders lease-guarded joined UPDATEs (edit count, supersede, clear pending freeze) through Dialect.JoinedUpdate.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.