Use the block-mysql driver

[morgo]

Part of the block/mysql#3 wave.

Why

strata links github.com/block/mysql, Block's fork of go-sql-driver/mysql, for capabilities upstream doesn't carry (QueryResultContext, Warnings()). That fork is moving from a replace directive to its own module path, because replace isn't inherited across module boundaries and so can't reach consumers of a library built on it.

Once the module path differs, the two packages declare distinct types. For spirit that is not a subtle problem — strata hands spirit a *mysql.Config directly:

targets = append(targets, applier.Target{
    DB:       db,
    Config:   cfg,   // *mysql.Config
    KeyRange: spiritKeyRange(spec.shard),
})

so without this change strata simply does not compile:

pkg/stratac/spirit_targets.go:62:14: cannot use cfg (variable of type
*"github.com/block/mysql".Config) as *"github.com/go-sql-driver/mysql".Config
value in struct literal

applier.Target.Config is the one strata hits, but it is not the only exported surface carrying a driver type — a consumer sizing its own migration should also expect:

Declaration	Type
applier.Target.Config	*mysql.Config
check.SourceResource.Config (pkg/move/check)	*mysql.Config
dbconn.UnsafeWarningError.Warning	*mysql.MySQLError

All three break loudly at compile time. Separately, a binary linking both packages carries two *mysql.MySQLError types, across which errors.As returns false silently rather than failing loudly — UnsafeWarningError.Warning is reachable by anyone classifying spirit's warnings with errors.As.

One break the compiler does not catch

EnhanceDSNWithTLS takes a string and returns a string, so nothing about it is typed. initRDSTLS registers "rds" in block/mysql's package-global TLS registry, and the returned DSN names that entry. A consumer that opens the result with upstream go-sql-driver now fails at connect time:

invalid value / unknown config name: rds

with nothing in the message about drivers:

$ go run ./tlscheck   # links upstream go-sql-driver only
upstream ParseDSN("u:p@tcp(mydb.cxyz.us-east-1.rds.amazonaws.com:3306)/app?tls=rds")
  err=invalid value / unknown config name: rds

block/schemabot is the live case. pkg/mysqlconn.ConnectionDSN calls EnhanceDSNWithTLS, and mysqlconn.Open then opens the result with sql.Open("mysql", …). It imports github.com/go-sql-driver/mysql only for ParseDSN, and none of the three driver-typed declarations above, so it would build clean, deploy, and fail on the first RDS dial after bumping spirit past this commit.

Consumers that call EnhanceDSNWithTLS and open the result themselves must open it with block-mysql. pkg/dbconn now exports DriverName for exactly this (sql.Open(dbconn.DriverName, …)), which stays correct through any future driver move.

What's in it

Mechanical, 123 files: the import path, and sql.Open("mysql", …) → sql.Open("block-mysql", …). The DSN format, Config, and the rest of the API are unchanged — the fork tracks upstream. Comments describing upstream driver behaviour still say go-sql-driver, which stays accurate since the fork inherits it.

One unrelated line: pkg/change/gtid_test.go gets a // sadscan:disable sq.pii.cc.visa. The GTID source id 11111111-2222-3333-4444-555555555555 contains a 16-digit run starting with 4, which the Visa PAN rule matches. It's a MySQL server UUID, not card data. The line is pre-existing and untouched by this change, but staging the file surfaced the finding.

dbconn.DriverName

The bulk of this diff is a literal string repeated at 259 call sites, so pkg/dbconn now exports the name and the production opens use it. Tests still carry the literal (or, in pkg/testutils, a private copy — testutils cannot import dbconn, since dbconn's own tests import testutils), which keeps the diff reviewable by inspection while giving external consumers something stable to open with.

Verification

• go build ./..., go vet ./..., gofmt -l all clean
• Full test suite passes against MySQL 8.0.44 — 33 packages, 0 failures

🤖 Generated with Claude Code

[aparajon]

🤖 Adversarial correctness review — 2d412cd1 (+378/-376, 123 files)

A 123-file find/replace is only reviewable by inspection if you can show it is one, so I checked that before reading anything. Every added line in the diff outside an import and a sql.Open is the three-line sadscan comment, and the only removed line outside those is the sidA const it replaces:

$ git diff main...HEAD -- '*.go' | grep '^+' | grep -v '"github.com/block/mysql"' | grep -v 'sql.Open("block-mysql"'
+		sidA = "11111111-2222-3333-4444-555555555555" // sadscan:disable sq.pii.cc.visa
+		// Visa PAN; it is a synthetic GTID source id, not card data.
+		// MySQL server UUIDs. sadscan reads the 4444-5555... digit run as a

So nothing was swept up by the replace — no schema name, config key or mysql system-database literal turned into "block-mysql", which is the failure mode a diff this wide usually hides. Zero sql.Open("mysql" remain, and the 16 surviving go-sql-driver strings are all comments about upstream behaviour, which stays accurate exactly as the body says.

What I'd want before this merges is a consumer-facing break that doesn't fail at compile time — and it happens to be the one that hits SchemaBot.

#	Sev	Where	What
1	med	pkg/dbconn/conn.go:456, PR body	EnhanceDSNWithTLS returns a DSN whose tls= name now lives in block/mysql's registry, so a consumer that opens it with upstream go-sql-driver fails at connect time — with an error that says nothing about drivers
2	low	pkg/lint/load.go:20	The one production sql.Open in the module that no test exercises: mutating it back to "mysql" passes the entire suite
3	low	PR body, "Why"	The compile-time break list names one of three exported driver-typed surfaces

---

1 — the TLS registry crosses the module boundary, and this is the exit the compiler doesn't guard (med)

The body's argument is about type identity, and for the surfaces it names that's right: strata stops compiling, loudly, which is the good failure. EnhanceDSNWithTLS is different because nothing about it is typed — it takes a string and returns a string.

initRDSTLS calls mysql.RegisterTLSConfig(rdsTLSConfigName, …) (conn.go:172), which after this PR writes into block/mysql's package-global registry, and EnhanceDSNWithTLS hands the caller back a DSN that refers to that entry by name. The name only resolves in the registry of the driver that owns it. I ran it both ways:

spirit v0.17.0 (main)     enhanced="…rds.amazonaws.com:3306)/app?tls=rds"   upstream ParseDSN err=<nil>
spirit @2d412cd1 (this)   enhanced="…rds.amazonaws.com:3306)/app?tls=rds"   upstream ParseDSN err=invalid value / unknown config name: rds

It is the only exported function in pkg/dbconn with that shape — newDSN and addTLSParametersToDSN are unexported, and New / NewWithConnectionType open the connection themselves, so they stay self-consistent.

The live consumer is block/schemabot. pkg/mysqlconn.ConnectionDSN calls dbconn.EnhanceDSNWithTLS, then Open does sql.Open("mysql", …) and OpenReloadable does sql.Open("mysql-hotswap-dsn", …). Both break on RDS hosts (tlsModeForHost enhances only RDS) the moment schemabot bumps spirit past this commit; it's on v0.16.1-… today. And schemabot imports none of the driver-typed API — not applier, not move/check — so the compile-time canary the body relies on never fires for it. It builds clean, deploys, and fails when it dials a database.

The consumer-side fix is also not as simple as "move to the fork", which is the part most worth writing down: mysql-hotswap-dsn is github.com/go-mysql/hotswap-dsn-driver, which imports upstream directly and builds its connector with upstream's mysql.NewConnector / mysql.ParseDSN. So that path stays on upstream until someone forks or patches it, and the consumer has to either stop using EnhanceDSNWithTLS or register the TLS config itself.

Two things would close this, both small on your side:

• Say it. One line in the body or the release note: consumers that call EnhanceDSNWithTLS and open the result themselves must open it with block-mysql. This costs an afternoon if it's discovered from invalid value / unknown config name: rds in a deploy instead of read in a changelog, because nothing in that string points at a driver.
• Export the driver name. The body files dbconn.DriverName under "follow-up worth considering / out of scope", framed as saving a future one-line diff. It's more than that — it's the affordance a consumer needs to open a spirit-produced DSN against the registry spirit wrote to. const DriverName = "block-mysql" in pkg/dbconn costs one line here and turns the consumer-side fix into sql.Open(dbconn.DriverName, …), which stays correct through the next driver move without every consumer having to know the name. Pulling just the constant forward leaves the 259-site cleanup as the follow-up and keeps this diff a pure find/replace.

The mirror direction deserves a sentence too, since it's the same split running the other way: a consumer that registers its own TLS config with upstream and hands a tls=<name> DSN to dbconn.New now fails identically. SchemaBot does register mTLS with upstream in pkg/engine/planetscale, but that path opens its own connections and never routes through spirit, so it's fine today — it just wouldn't survive someone wiring the two together.

2 — one production call site has no test behind it (low)

// pkg/lint/load.go
func LoadSchemaFromDSN(ctx context.Context, dsn string) ([]*statement.CreateTable, error) {
	db, err := sql.Open("block-mysql", dsn)

I mutated this one line back to "mysql" and ran the whole suite in the compose harness: 33 packages, 0 failures. Not pkg/lint alone — the entire module passes with a production sql.Open naming a driver that isn't registered.

The site itself is correct in this PR, so this isn't a defect in the diff. It's a statement about what the diff's correctness rests on: for every other call site a miss would have been caught by a test, and for this one it wouldn't. LoadSchemaFromDSN is exported and reached from cmd/spirit's lint and diff subcommands, so a miss here ships and surfaces the first time someone runs spirit lint --dsn …. Loudly, at least — sql: unknown driver "mysql" (forgotten import?) — but from a release rather than from CI.

Either a small test that calls LoadSchemaFromDSN against the compose MySQL, or the dbconn.DriverName constant from finding 1, closes it. The constant is the better answer, because a constant can't be half-replaced — that's the third independent argument for it in this review.

3 — the "Why" section undercounts the compile-time surface (low)

The body names applier.Target.Config, which is the one strata hits. Two more exported declarations carry driver types and change identity with the module path:

pkg/move/check/check.go:32     Config  *mysql.Config          (check.Check)
pkg/dbconn/dbconn.go:144       Warning *mysql.MySQLError      (dbconn.UnsafeWarningError)

Both break loudly, so this isn't a safety problem — it's that a consumer reading the body to size its own migration will size it wrong, and UnsafeWarningError.Warning in particular is reachable by anyone classifying spirit's warnings with errors.As. Three lines instead of one.

Same category, not its own row: the sadscan directive is in the canonical form used elsewhere in the org (// sadscan:disable <rule-id>, with an optional -- reason), and putting the reasoning in a comment above reads better than the trailing form. I couldn't verify that sq.pii.cc.visa is the exact rule id, and a mistyped id suppresses nothing while looking like it does — worth confirming the finding is actually gone on the next scan rather than assuming.

---

Verified — the suite, the probes, the pin, and three attacks that dissolved

Local: go build ./..., go vet ./... and gofmt -l clean. Full suite via compose the way CI runs it (MySQL 8.0.45, -race -parallel=4): 33 packages, 0 failures, which matches the body. CI is green on all seven build jobs plus gtid/nogtid, lint, govulncheck and the generated-parser check.

A missed call site fails loudly, module-wide. I probed two test binaries rather than one, because sql.Drivers() is per-binary and pkg/change pulls in a lot of go-mysql-org/go-mysql:

pkg/dbconn   PROBE drivers=[block-mysql]  open_mysql_err=sql: unknown driver "mysql" (forgotten import?)
pkg/change   PROBE drivers=[block-mysql]  open_mysql_err=sql: unknown driver "mysql" (forgotten import?)

Attack that dissolved: a second driver registering "mysql". github.com/go-mysql-org/go-mysql/driver does register the name mysql, and spirit depends on that module — so a missed sql.Open("mysql", …) could have silently bound to a completely different implementation instead of failing. It doesn't: that package is not in the build graph (go list -deps ./...), only client, replication, mysql, packet and friends are. This was the one way the rename could have gone wrong quietly, and it's closed.

Attack that dissolved: go-sql-driver is still in go list -m all. It shows up at v1.7.1 even though the direct require and both go.sum lines are gone. go mod why -m github.com/go-sql-driver/mysql answers "main module does not need module …" — a pruned-graph entry from a dependency's go.mod, with no package linked, consistent with go list -deps and with both probes.

Attack that dissolved: the pin. github.com/block/mysql v0.0.0-20260906201522-a3178f8dca69 resolves to a3178f8d, the squash-merge of block/mysql#3 and the current head of that repo's master, so nothing here depends on unmerged code. Worth knowing in the other direction: block/polt#24 and block/vitess#22 both pin spirit at this branch head, so this PR is the gate for re-pinning them.

Leak check: clean. The sadscan directive puts an internal scanner's rule id in a public repo, which is the only thing in the diff that reads internal — but sadscan:disable already appears 47 times across public block org code, so it discloses nothing new. Nothing else in the diff or body is internal; strata is already named in public spirit and vitess source.

Both probe files were moved out of the tree and the mutation restored from backup; the worktree is clean at 2d412cd1.

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

[aparajon]

🤖 Approving. The find/replace is provably exhaustive and the suite is green (33 packages, 0 failures locally). The one thing worth acting on before merge is in the review above: EnhanceDSNWithTLS now hands back a tls=rds DSN registered in block/mysql's registry, which breaks block/schemabot's mysqlconn at connect time when it bumps — reproduced with a control against main. Pulling dbconn.DriverName forward from the follow-up section is the cheap fix.

This stamp was left by Claude Code (claude-opus-5).