feat(storage): route direct pool execution through rebind-aware DB wrapper

[Kiran01bm]

Summary

Routes all direct connection-pool execution in the shared sqlstore core through a rebind-aware DB wrapper. Every statement executed on the pool now passes through the dialect's placeholder binder exactly once before reaching the SQL driver — the identity transform for MySQL, and the seam a Postgres backend needs to rewrite ? placeholders to $n without touching store SQL.

What

• New rebindDB wrapper around *sql.DB: ExecContext / QueryContext / QueryRowContext rebind the query via a binder interface; BeginTx / Conn / PingContext / Close are raw passthroughs.
• MySQLDialect gains an identity Rebind.
• All store structs (and Storage) hold *rebindDB instead of *sql.DB, so no store can bypass the rebind boundary. New(db *sql.DB) is unchanged; no SQL or behavior edits.
• Unit tests back the wrapper with a recording driver.Connector stub, proving each direct execution method rebinds exactly once and forwards the rebound SQL and args, and that Tx/pinned-conn handles bypass the binder.

Why

The store core emits MySQL-style ? placeholders throughout. Postgres requires $n placeholders at the wire, and rewriting ~120 hand-written statements would be invasive and error-prone. Centralizing the rewrite at the execution boundary keeps store SQL dialect-neutral: a Postgres constructor only needs to supply a ?-to-$n binder. Transaction and pinned-connection statement rebinding is intentionally deferred to a follow-up to keep this change narrow.

Before / after

Before:
  store ──── query with "?" ────────────────────▶ *sql.DB ──▶ driver

After:
  store ──── query with "?" ──▶ rebindDB ──▶ binder.Rebind (MySQL: identity)
                                    │
                                    └───── rebound query ──▶ *sql.DB ──▶ driver

  BeginTx / Conn: raw passthrough handles (no rebinding yet)

[aparajon]

🤖 Adversarial correctness review, requested by Armand and performed by his agent. Reviewed at head 79dc872 (the PR's own diff on top of #948's package move).

Verdict: clean — no fix-before-merge findings. The seam is in the right place, the boundary is actually sealed (I went looking for bypasses and found none), and the identity transform provably changes nothing for MySQL.

Verified solid:

• The "no store can bypass the rebind boundary" claim holds under inspection, not just by convention. After this change the only raw *sql.DB left in the package is the wrapper's own field/constructor and the New(db *sql.DB) entry point; every store struct holds *rebindDB, and nothing outside db.go touches .pool. Since rebindDB deliberately exposes no accessor for the underlying pool, a future store literally cannot execute pool SQL around the binder without changing db.go itself — the boundary is structural.
• The identity.InsertID path routes through the seam too. queryExecer is satisfied by both *rebindDB (pool path — rebound) and *sql.Tx (tx path — raw, exactly matching the documented BeginTx/Conn passthrough), so the guarded-insert helpers get the same one-rebind guarantee as direct executions.
• The raw-handle carve-out is coherent and honest. BeginTx/Conn return raw handles, the godoc says so, and the follow-up deferral is explicit in the PR body. The handles that matter today are MySQL-only anyway (the GET_LOCK pinned-conn path and the tx-based claim machinery), so nothing currently reachable executes un-rebindable SQL on a non-MySQL engine.
• The driver-stub tests prove the right things at the right layer. The recording driver.Connector observes the exact SQL text handed to the driver, so the exactly-once rebind, the args forwarding, and — the one that usually goes untested — the negative case (tx and pinned-conn statements reach the driver with native placeholders and zero binder calls) are all pinned below database/sql, not by mocking the wrapper itself.
• No behavior change for MySQL, proven end to end: the identity Rebind plus the full sqlstore integration suite running every store's real SQL through the wrapper against MySQL — green locally with -race, and CI is green 32/32.

One forward-looking note for the follow-up (nothing to change here): the binder contract says "pure string transformation", which is right, but the eventual ?→$n Postgres binder must also skip ? inside string literals and comments — the store's plain-statement SQL style makes that tractable, and the parity suite from #935 is the natural place to pin it when it lands.

This review was generated by Claude Code (claude-fable-5).

[aparajon]

🤖 Approving per the adversarial review above — clean, no blocking findings. Stamped by Armand's agent (claude-fable-5).

[chatgpt-codex-connector]

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