feat(engine): capture per-table size estimates at plan time

[aparajon]

Why this matters

A plan shows the DDL but not the scale of what it touches: ADD INDEX on a 12-row table and on a 120-million-row table read identically, so the operator judging "how long will this index build run" has nothing to go on. The same missing data hurts apply progress from the other side (#1183, #1191): shards the driver cap has not dispatched yet report no row totals, so the table-level copy bar can only disclose partial coverage instead of estimating the whole table.

What is captured

Four new optional fields on TableChange, one set per planned table change:

Field	Source	Example
EstimatedRows	TABLE_ROWS, summed across shards (Spirit, Strata)	13_100_000
EstimatedBytes	DATA_LENGTH + INDEX_LENGTH, summed (Spirit, Strata)	6_200_000_000
ShardCount	planned shards the change spans	4
LargestShardRows	biggest single shard's rows — the largest chunk a shard-at-a-time apply works through at once (Spirit, Strata)	3_400_000

All values come from engine statistics: approximate, possibly stale, display-only. Nothing reads them yet — the plan-comment rendering is #1230.

ShardCount zero means there is no shard count to render — the target is not sharded, or its topology could not be read. The two are indistinguishable, so a renderer omits the count rather than asserting "not sharded".

When and how

Captured once, at plan time, while the engine computes the diff — never re-probed at apply. Spirit and Strata share one statistics query, scoped to the tables the plan touches:

SELECT table_name, table_rows, data_length + index_length
FROM information_schema.tables
WHERE table_schema = DATABASE() AND table_type = 'BASE TABLE'
  AND table_name IN (?, ?, ...)

• Spirit runs it once, on the differ's own connection.
• Strata plans each shard on that shard's own connection, so the query runs once per shard inside the plan it already makes; the core folds the per-shard results into one namespace-level TableChange. A shard contributes each of its tables once even when it plans several statements against that table, so the shard count counts shards.
• PlanetScale has no database connection at plan time and reads its sizes from the API instead, which is a separable concern with its own attribution reasoning: feat(engine): capture PlanetScale table byte estimates at plan time #1249.

per-shard probes            aggregation                     persisted
 -80:   3.4M rows/1.6GB      sum rows & bytes                proto (tags 10–13)
 80-c0: 3.2M rows/1.5GB  ─▶  keep largest single shard  ─▶   storage
 c0-e0: 3.1M rows/1.5GB      count planned shards            apitypes
 e0-:   3.4M rows/1.6GB
                             = TableChange{EstimatedRows: 13.1M, EstimatedBytes: 6.2GB,
                                           ShardCount: 4, LargestShardRows: 3.4M}

Guardrails

• Estimates never gate anything and never fail a plan. A failed probe logs a warning and the plan proceeds without sizes; the direct-execution size gate is untouched.
• All-or-nothing aggregation. If any shard's estimate is missing, the summed totals are omitted entirely and only the shard count survives — the example above with one dead probe becomes TableChange{ShardCount: 4}, because a partial sum would silently understate the table.
• Statistics, not scans. TABLE_ROWS / DATA_LENGTH are cached dictionary statistics; the server answers from metadata it already has, at any table size.
• Unconditional, but scoped. The probe is not gated on whether the DDL's cost scales — feat(ddl): detect statements whose cost scales with table size #1237's classifier gates only the rendering in feat(github): show table-size estimates in the plan comment #1230, and the estimates also feed the future progress denominator regardless of statement shape. It is scoped to the planned tables, so its cost does not grow with the size of the schema around them.
• A hard probe budget. TableSizeProbeTimeout (5s) bounds Spirit's single query and the PlanetScale metrics call; on expiry the probe is abandoned and the plan proceeds without sizes, so a wedged statistics read can never extend plan latency past the budget.

How it moves us toward the northstar

Third slice of the plan-time table-size estimates stack (#1236 formatters → #1237 cost-scaling detection → this PR → #1249 PlanetScale capture → #1230 plan-comment rendering). Persisted with the plan, these estimates can later seed the progress denominator for shards whose waves have not started, turning #1191's "across N of M shards" disclosure into a whole-table bar.

Opened by Claude (Fable 5).

[Copilot]

Pull request overview

This PR adds best-effort, plan-time per-table size estimates (rows + bytes) to engine.TableChange, aggregates them correctly across shards for sharded plans, and plumbs the new fields through proto/storage/API layers so they persist with the plan and are available for later rendering/UX work.

Changes:

• Capture plan-time size estimates in engines (Spirit: information_schema; PlanetScale: API shard counts + vtgate shard probes when DSN is available).
• Aggregate per-shard estimates into namespace-level totals (sum + largest shard) with “all-or-nothing” semantics when any shard is missing an estimate.
• Extend proto/storage/apitypes models and conversion helpers, and add tests covering aggregation and engine-side estimate capture.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/engine/engine.go	Adds new optional size-estimate fields to the engine-level TableChange contract.
pkg/engine/spirit/spirit.go	Queries information_schema for per-table rows/bytes at plan time and attaches estimates to planned changes (best-effort, non-fatal).
pkg/engine/spirit/spirit_integration_test.go	Integration test validating Spirit attaches estimates for existing tables and omits them for newly created tables.
pkg/engine/planetscale/plan.go	Fetches shard counts (best-effort) and attaches keyspace table-size data onto planned changes.
pkg/engine/planetscale/table_sizes.go	Implements PlanetScale shard-count lookup and vtgate shard probes for per-table rows/bytes, aggregated across shards.
pkg/engine/planetscale/table_sizes_test.go	Unit tests for shard-count lookup and DSN-absent behavior (counts still attached, estimates omitted).
pkg/tern/local_client.go	Computes shard-level aggregates and overwrites the namespace view’s kept TableChange with cross-shard totals.
pkg/tern/table_sizes.go	New accumulator implementing shard aggregation with “all-or-nothing” totals when any shard is missing.
pkg/tern/tablechange_convert.go	Plumbs new fields through engine→storage, engine→proto, and proto→storage conversions.
pkg/tern/local_client_shardplan_test.go	Tests namespace aggregation behavior, missing-shard behavior, and pass-through behavior for already-aggregated changes.
pkg/proto/tern.proto	Adds new TableChange fields (estimated_rows, estimated_bytes, shard_count, largest_shard_rows).
pkg/proto/ternv1/tern.pb.go	Regenerates Go proto bindings for the new fields and accessors.
pkg/storage/types.go	Persists new estimate fields on stored storage.TableChange.
pkg/apitypes/apitypes.go	Exposes new estimate fields in API TableChangeResponse.
pkg/api/proto_helpers.go	Maps proto TableChange fields to API response struct (including new estimate fields).

Files not reviewed (1)

• pkg/proto/ternv1/tern.pb.go: Generated file

---

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

[morgo]

🤖 Approving on Morgan's behalf (automated review).

The parts that could have gone wrong here mostly didn't. The plan-time probe is genuinely best-effort — its own TableSizeProbeTimeout (5s), failure sets sizeEstimates = nil and the plan proceeds, and a NULL statistics row is skipped rather than recorded as zero. Proto fields 10–13 are cleanly additive past the previous max of 9, no reuse and nothing needing reserved. And the "never a gate input" instruction is stated in both the proto and the Go doc, which is the right guardrail for a stale-by-nature number.

tableSizeAccumulator handles the trap I went looking for: a partial sum across shards would understate a table, and the missing flag makes row totals all-or-nothing while letting shardCount survive. That's the correct call and it's explained in place.

CI green. Two findings, one worth acting on.

1. The probe reads every base table in the schema to use a handful.

SELECT table_name, table_rows, data_length + index_length
FROM information_schema.tables
WHERE table_schema = DATABASE() AND table_type = 'BASE TABLE'

Only planned tables are ever read back out of the map — sizeEstimates[pc.TableName], and pc comes from plan.Changes, which is already in hand before the probe runs.

On MySQL 8.0, TABLE_ROWS / DATA_LENGTH / INDEX_LENGTH are dynamic columns: when the cached statistics have aged past information_schema_stats_expiry (default 24h), the server retrieves them from the storage engine, opening each table to do it. On a schema with thousands of tables that is a genuinely expensive query, and it now runs on every Plan — i.e. every PR push that triggers a plan — against the production database.

The 5s bound means it can't stall a plan, which is why this isn't a hold. But the degenerate case is quiet: a large schema pays 5s on every plan, gets nothing, and leaves a warning in the log. Narrowing to the planned tables would cut the work proportionally and make the timeout far less likely to be reached:

... WHERE table_schema = DATABASE() AND table_type = 'BASE TABLE'
    AND table_name IN (?, ?, ...)

Same cost model as today when a plan touches every table, dramatically cheaper in the normal case of a plan touching two or three.

Minor, same area: binding the schema explicitly instead of DATABASE() would fail loudly rather than return zero rows if a DSN ever arrives without a default database selected. Existing code (fetchCurrentSchema) leans on the DSN the same way, so this is consistency rather than a defect.

2. shardCount and sum count TableChange entries, not distinct shards.

In aggregateShardTableSizes, a.shardCount++ and a.sum += *tc.EstimatedRows fire once per tc in sc.TableChanges. If a single sharded SchemaChange can ever carry two TableChanges naming the same table — two statements against one table in the same namespace — that shard is counted twice and its rows are added twice, inflating both the shard count and the total.

I could not establish from this diff whether the planner guarantees one TableChange per table per SchemaChange, so I'm raising it as a question rather than asserting a bug. If that uniqueness is guaranteed upstream, it's worth a line in the accumulator's doc comment saying so, since the fold silently depends on it. If it isn't, keying the accumulator on the shard name rather than incrementing per entry would make it robust either way.

Cosmetic, ignore freely: two doc comments have mangled line wrapping mid-sentence — LargestShardRows in engine.go (// Nil alone on a line) and the aggregateShardTableSizes header (// the number of / // planned shards). Presumably a formatter artifact.

[Kiran01bm]

🤖 Review findings - created by Kiran's code review agent - for schemabot/pull/1238, 2585671.

Verdict: 8 findings — 3 blocking (shard overcount, keyspace-unscoped byte map, dropped API hop), 5 non-blocking. Every finding is latent today (nothing renders these fields yet), which is exactly why they should be fixed now rather than debugged through #1230.

Blocking

1. aggregateShardTableSizes counts table-change entries, not shards — inflating ShardCount and doubling the row/byte sums.
pkg/tern/table_sizes.go:61-77

for _, tc := range sc.TableChanges {
    a := byTable[tc.Table]
    ...
    a.shardCount++            // once per TableChange, not once per shard
    a.sum      += *tc.EstimatedRows
    a.bytesSum += *tc.EstimatedBytes

One shard's SchemaChange can legitimately carry the same table more than once — a diff may emit several statements for one table (a partition-type change needs REMOVE PARTITIONING then PARTITION BY), and the engines append one engine.TableChange per planned statement. The enclosing function already knows this: local_client.go:1544-1546 documents that the namespace view "collapses them (deduping repeated tables)", and :1580 does exactly that. But the fold runs at :1551, over the undeduped result.Changes. So the repeats the dedupe exists to remove are counted N times, and the single surviving row carries the inflated aggregate.

Proved by execution via an injected go test -overlay probe (no tracked file touched) — 2 shards × 2 statements for one users table, 100 rows / 10 000 bytes each:

PROBE RESULT: shard_count=4 estimated_rows=400 estimated_bytes=40000 largest_shard_rows=100
TRUTH:        want shard_count=2 rows=200 bytes=20000 largest=100

LargestShardRows stays correct because max is idempotent — which makes it harder to spot, since sum being 2× while largest is right looks like a plausible topology rather than a bug. Failure: a partition-type change on a 4-shard keyspace renders "spans 8 shards, ~2× the rows and bytes". Same defect at the storage seam (:1990/:2008). Fix: dedupe sc.TableChanges by table before folding, or key the increment on a (shard, table) seen-set.

Latent today — no in-tree engine sets Shard at plan time, so this fold only serves out-of-tree Strata. But that is precisely the path the PR body names as its motivating case, and this is the PR's central new logic for it.

2. The PlanetScale byte map is fetched branch-wide but applied per keyspace, dropping an invariant the PR itself deleted.
pkg/engine/planetscale/plan.go:139

attachTableSizes(shardCounts[ks], tableBytes, tableChanges)

shardCounts is subscripted by ks; tableBytes is passed whole. One flat, bare-table-name map (:108, commented "one call covers every keyspace on the branch") is reused unqualified for every keyspace, and table_sizes.go:46 looks up tableBytes[changes[i].Table].

The repo defends (namespace, table) as the table identity in two production helpers, both commented as to why: shardTableKey{namespace, shard, table} (local_control_resume.go:489 — "the same table name can appear in more than one namespace (multiple Vitess keyspaces)… keying by table name alone would conflate tasks") and progressTableKey (progress_handlers.go:125 — "Vitess applies commonly include the same table name in multiple keyspaces"). Decisively, the code this PR replaced was rigorously scoped: fetchKeyspaceTableRowEstimates(ctx, dsn, keyspace) built a fresh per-keyspace map and carried an explicit guard ("SHOW VITESS_SHARDS lists every keyspace; other keyspaces' shards are not this probe's concern"). Hoisting the map above the loop did not re-establish that scoping.

It fails either way, and which way is unverifiable from the repo: if the API keys are bare names, two keyspaces sharing a config/orders/audit_log table both get stamped with one value and ok is true for both, so nothing warns; if the keys are qualified, every lookup misses and the feature is a permanent silent no-op indistinguishable from "no metrics available". Nothing tests it end to end — the only evidence for the assumed shape is the PR's own fixture (client_test.go:161), and localscale implements no /metrics/tables route. Worth checking whether PlanetScale's keyspace-grouped variant of this endpoint is the right one to call.

3. tableChangeResponseFromStorage drops all four new fields — the storage→apitypes hop was missed while the proto→apitypes sibling was swept.
pkg/api/plans_handlers.go:257-268

func tableChangeResponseFromStorage(change storage.TableChange) *apitypes.TableChangeResponse {
	return &apitypes.TableChangeResponse{
		TableName:     change.Table,
		Namespace:     change.Namespace,
		DDL:           change.DDL,
		ChangeType:    change.Operation,
		IsUnsafe:      change.IsUnsafe,
		UnsafeReason:  change.UnsafeReason,
		ExecutionMode: change.ExecutionMode,
		ModeReason:    change.ModeReason,
	}
}

The four sibling advisory fields are carried in the same literal; the four new ones are absent. The sibling converter proto_helpers.go:178-181 was updated. I checked the strongest refutation — that nothing is lost because nothing persists — and it fails: local_client.go:2007 populates the stored struct, storage.TableChange carries JSON tags, persistence is a plain whole-struct json.Marshal into plan_data with no custom marshaller, and the read-back unmarshals intact. The loss is purely at this hop. Live route: GET /api/plans/{plan_identifier} (service.go:779) → storedPlanResponseFromStorage → planContentFromStorage, plus schemabot plans show via GetStoredPlan.

Consequence once #1230 lands: the same plan shows sizes in a fresh PR comment and blank in the stored-plan view. Note that the design comment in tablechange_convert.go:3-9 — "a helper that stops compiling is the signal that a boundary was missed" — cannot protect this hop, because it lives in pkg/api outside that helper set and adding a struct field never breaks compilation.

Non-blocking

4. fetchKeyspaceShardCounts is unbounded while its sibling ten lines later is budget-bound.
plan.go:97 passes the raw plan ctx; plan.go:107 wraps its call in context.WithTimeout(ctx, engine.TableSizeProbeTimeout). Both are new, both display-only, both best-effort. The PlanetScale SDK's default client leaves http.Client.Timeout at zero, so a stalled keyspaces endpoint blocks Plan for as long as the caller allows — for a shard count the code is explicitly willing to discard. This also makes engine.go:341-344 ("a slow or wedged statistics read must never extend plan latency past this bound") not quite true of everything the PR added. createBranch already bounds a comparable call at branch.go:474.

5. ShardCount's documented contract contradicts what the code produces, in both directions.
engine.go:380-381 says "Zero when the target is not sharded or the shard topology is unknown". But table_sizes.go:33 reads counts[ks.Name] = ks.Shards and ignores the SDK's Sharded bool — an unsharded keyspace reports Shards: 1 (the repo's own emulator encodes this: handlers_schema.go:40-47, if shards == 0 { shards = 1 } … Sharded: shards > 1). So unsharded yields 1, not 0; and 0 arises only from a missing key or the nil map after a ListKeyspaces failure — i.e. it means "unknown" and never "unsharded". A renderer trusting the doc prints "not sharded" for a 32-shard keyspace whose lookup timed out. Either consult ks.Sharded, or change the comment so 1 means unsharded and 0 means unknown — the code and comment cannot both be right.

6. missing couples rows and bytes, and the doc comment only admits half of it.
table_sizes.go:68 sets missing if either estimate is nil, and sizes() then returns nil for bytes too — but the comment at :18-21 promises only that "the row values are omitted entirely while the shard count survives". A bytes-only engine therefore loses everything, which matters because this PR introduces exactly such a source (psclient documents "the endpoint reports bytes only — no row counts"). No test pins the coupling: a mutant setting missing on nil rows only, and summing bytes opportunistically, survives the entire pkg/tern suite.

7. The new Spirit integration test leaks tables into the shared container.
spirit_integration_test.go:2314 creates sized_items and sized_gadgets but never calls cleanupTables(t, db), against the helper's stated contract ("Each test should clean up its own tables") that ~14 sibling tests honour. With DEBUG=1 the container is reused, so the second run fails at CREATE TABLE sized_items. Blast radius is small only by position — the test is last in the file — so any test appended after it that plans all of testdb will see two undeclared tables and plan them as DROP TABLE.

8. Every localscale-backed plan now logs an ERROR for a display-only feature. localscale's route table has no /metrics/tables, and Go 1.22 patterns are segment-exact, so the request lands on the catch-all 501 at server.go:1393. That produces an slog.Error from doRawJSON plus the engine's own Warn — an ERROR-level line on a correctly configured local run. Add the route, or downgrade the raw-HTTP log for best-effort reads.

General suggestions

• Two doc comments have stray mid-sentence line breaks: // Nil alone on engine.go:385, and // number of / // planned shards in table_sizes.go:42-43.
• plan.go:138's if len(tableChanges) > 0 guard is redundant — attachTableSizes already no-ops on an empty slice.
• tableMetricsServer was inserted between autoCutoverServer's doc comment and its function (client_test.go:143), so that comment now documents the wrong function and autoCutoverServer has none.
• fetchTableSizeEstimates accepts a negative TABLE_ROWS while direct.go:124-129 rejects it as a sentinel. I chased this hard and it is unreachable — TABLE_ROWS is bigint unsigned on the pinned mysql:8.0.44, and the driver converts an out-of-int64 unsigned to a string on both protocols, so a Scan error (already handled fail-soft) results rather than a negative. Belt-and-braces only.
• EstimatedBytes is summed but has no largest_shard_bytes counterpart while rows get both; worth a note on why the asymmetry is intended.
• The PR adds two unconditional, serial PlanetScale round trips before any diffing, on every plan of every PR sync, against an org-wide rate limit — even when the plan turns out to have zero changes.
• The size probe reads MySQL's cached dictionary statistics (default information_schema_stats_expiry 86400s) while direct.go deliberately disables that cache. I verified this is correct as designed, not an oversight — see below — but if feat(github): show table-size estimates in the plan comment #1230 ever renders an estimate next to a size verdict, that's the PR to reconcile it on.

The one thing that could have broken, verified

The all-or-nothing nil contract in sizes(). If a partial sum ever leaked out, a plan would understate a table's size and an operator could green-light a change against a wrong mental model — a silent, plausible-looking wrong number is the worst failure this feature can produce. I mutated sizes() to return the partial sum unconditionally; it was killed immediately by TestPlanShardTableSizesOmittedWhenAnyShardMissing at four separate assertion sites (local_client_shardplan_test.go:271,272,276,277 — "Expected nil, but got: (*int64)"), across both the storage and proto paths. A control mutant (shardCount + 100) confirmed the harness actually reaches the code, dying at four more sites with "expected: 2, actual: 102". The contract holds; finding 1 is a defect in what gets folded, not in the nil semantics.

Verified correct

• Proto tags 10-13 are collision-free and additive. message TableChange uses 1-9 with no reserved; the nearby reserved 7/8/9 belong to other messages. tern.pb.go regenerates consistently, honouring docs/release.md's "the gRPC contract is additive only".
• The staleness asymmetry with direct.go is deliberate and principled, not an oversight. The rule is "fresh iff it gates", stated at four layers (engine.go:369 "never a gate input", storage/types.go:366, spirit.go:1082, and the PR body). Applying direct.go's stats_expiry = 0 here would turn the unfiltered all-tables query into an open-every-table operation — an actual regression against the 5s budget. Both sites are right.
• A negative TABLE_ROWS cannot arrive over the wire — verified against the pinned mysql:8.0.44 image (TABLE_ROWS bigint unsigned) and both driver protocols.
• Persistence is intact end to end — whole-struct JSON into plan_data, no custom marshallers anywhere in pkg/storage, namespacesWithShardPlans copies whole structs.
• largest starting at 0 and only growing is correct (unsigned source, 0 is a valid floor); a smallest-instead-of-largest mutant was killed.
• Unsharded exclusion is correct and pinned — a mutant folding unsharded changes in was killed by TestPlanUnshardedTableSizesPassThrough; a nil inner map returns nil, so the engine's own values pass through untouched.
• Timeout and cancellation handling is sound on both probes — cancelProbe/cancelMetrics run unconditionally on every path, the child context never poisons the parent, and a nil map on failure is a legal zero-value read.
• Spirit's sizeEstimates[pc.TableName] lookup cannot miss on case — the upstream differ already joins current and desired schemas by byte-exact map key, so any table with an ALTER has a name byte-identical to information_schema's.
• No race in the concurrent attachTableSizes — diffKeyspace builds a fresh slice per keyspace, so the mutations are on disjoint slices and the shared maps are read-only.
• Integer conversions are safe — int32(shardCount) would need >2.1e9 shards; the int64 byte sum ~9.2 exabytes.
• CI is fully green at this head, including all E2E Vitess, K8s, MySQL and gRPC shards.

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

[morgo]

🤖 Re-approving on Morgan's behalf (automated review) — my stamp had gone stale at 2585671a, and the PR has grown from +73/-0 to +788/-40 since, so this is a fresh look rather than a rubber stamp.

Both findings from my earlier review are fixed, and correctly.

The probe no longer reads the whole schema (f16a8bd3). The query is now scoped to the planned tables:

... WHERE table_schema = DATABASE() AND table_type = 'BASE TABLE'
  AND table_name IN (?, ?, ...)

with plannedTableNames deduplicating first — and the dedup carries the right reason, that one table can legitimately carry several statements (a partition-type change needs its own REMOVE PARTITIONING). The len(tables) == 0 early return is load-bearing and present: without it strings.Repeat(", ?", len(tables)-1) would panic on a negative count. Cost now scales with the plan instead of the schema, which was the point.

The shard fold counts shards, not entries (0b32b685). seenShards gates the whole per-table accumulation, so a shard planning two statements against one table contributes one to shardCount and one set of values to the totals. That's the shard-keyed fix rather than a comment promising uniqueness, which is the more robust of the two options I offered.

The author also went past what I raised: missing is now split into missingRows and missingBytes, tracked independently. That's a real improvement — a source carrying bytes but no row estimates now still yields a byte total instead of having both suppressed by one absent kind.

On the new material (roughly 700 lines I hadn't seen):

• tablechange_convert.go and proto_helpers.go show deletions, but every one is gofmt realignment for the widened struct literals — no existing field's behavior changes. Worth confirming explicitly since a -24 on a converter is where I'd expect a regression to hide.
• The size aggregate is applied at both seams — planResultToProtoChanges for the live response and namespacesFromEngineChanges for the stored plan — using the same aggregateShardTableSizes fold. That's the consistency property that matters here: the numbers in a PR comment and the numbers in the stored-plan view are computed the same way and can't drift. tableChangeResponseFromStorage then reads the persisted values, so the stored-plan view doesn't re-derive anything.
• Attaching the cross-shard totals to the first-seen (kept) entry composes correctly with the now-merged #1243 dedup, which only collapses when sc.Sharded() — the same predicate aggregateShardTableSizes skips on. Unsharded changes keep the engine's own values untouched, as documented.
• Deferring the PlanetScale byte capture to its own change (0234f7d8) leaves nothing half-wired here — there are no pkg/engine/planetscale files left in the diff.

Full suite green (38 checks). Read-only, display-only, still bounded by TableSizeProbeTimeout with failure degrading to "no sizes" rather than a failed plan, so nothing here can hurt the live MySQL path.

Two residuals, both nits, neither worth a change on its own:

The IN match is exact, so a planned table name that differs from information_schema.tables.table_name in case (under a case-sensitive lower_case_table_names) or in qualification simply doesn't match and yields no estimate. That degrades in the right direction — a missing size line, not a wrong one — which is why it's a nit rather than a finding.

int32(shardCount) narrows an int at two call sites. Not reachable at any plausible shard count; noting it only because it's the kind of conversion that gets copied into a context where it does matter.

The staleness caveat on TABLE_ROWS is unchanged and correctly documented in place as display-only.

[aparajon]

🤖 Addressing morgo's review and Kiran's review. Both landed on the same defect from different directions, so they are answered together. origin/main is also merged in (57d11b4c) and the PR is mergeable again.

Finding	Status	Commit
The fold counts table-change entries, not shards (morgo 2, Kiran 1)	Fixed	0b32b685
missing couples rows and bytes (Kiran 6)	Fixed	0b32b685
Stored-plan view drops all four fields (Kiran 3)	Fixed	78c966d7
Branch-wide byte map applied per keyspace (Kiran 2)	Fixed, then moved to #1249	0208cdca
fetchKeyspaceShardCounts unbounded (Kiran 4)	Fixed, then moved to #1249	0208cdca
ShardCount's doc contradicts the code (Kiran 5)	Fixed	0208cdca
Probe reads every base table in the schema (morgo 1)	Fixed	f16a8bd3
Spirit test leaks tables into the shared container (Kiran 7)	Fixed	f16a8bd3
Redundant len(tableChanges) > 0 guard, doc wrapping, autoCutoverServer doc pairing	Fixed	0208cdca
Localscale has no /metrics/tables route (Kiran 8)	Moved to #1249	—
Bind the schema explicitly instead of DATABASE() (morgo, minor)	Declined	—

The fold counted entries, not shards. Morgan asked whether one sharded SchemaChange can carry two TableChanges naming the same table; it can — a partition-type change plans its own REMOVE PARTITIONING statement alongside the PARTITION BY, and #1243 (now merged in) exists precisely to keep both. So the count was inflated and the rows and bytes were double-added. The accumulator now records the shards it has seen per table and folds each once, which is robust regardless of what the planner guarantees upstream. TestPlanShardTableSizesCountShardsNotStatements plans two statements against one table on two shards and asserts ShardCount: 2 with unduplicated totals.

Rows and bytes are now independent. A single missing flag meant an engine that reports bytes without row counts — PlanetScale, exactly — had its byte total suppressed by the absent row estimates. Each kind is all-or-nothing on its own now, with TestPlanShardTableSizesKeepBytesWithoutRows pinning it. The doc comment also admits the asymmetry Kiran flagged: there is no largest_shard_bytes because the largest shard's rows bound the biggest chunk a shard-at-a-time apply works through at once, while bytes only convey magnitude, for which the total is the number an operator reads.

PlanetScale bytes are attributed, not guessed. Kiran is right that the branch metrics endpoint has no keyspace dimension, so its bare table names are only attributable on a single-keyspace branch. Rather than restore a "one keyspace only" guard around the attach, the read itself is now gated: a multi-keyspace branch skips the call entirely and logs why. That costs byte estimates on multi-keyspace PlanetScale databases, which I think is the right trade — a confidently wrong size is worse than an absent one — and it also removes one of the two round trips on exactly the branches where the answer would have been unusable. Separately, an unsharded keyspace now maps to no shard count instead of one, and ShardCount's doc says what zero means: there is nothing to render, because the target is not sharded or its topology could not be read, and a renderer must not turn the two into "not sharded".

The probe is scoped to the planned tables. plan.Changes is in hand before the probe runs, so the query now binds the planned table names and dedupes them first. Same cost as before when a plan touches every table, proportionally cheaper in the normal case. I left DATABASE() in place: fetchCurrentSchema on the same DSN leans on it the same way, and changing one of the two would make the pair inconsistent without fixing anything.

The PlanetScale path now lives in #1249. Kiran's finding 2 is what prompted it: attributing a branch-scoped, keyspace-less byte map is its own piece of reasoning, and it does not belong in the same change as reading statistics off a connection. #1249 carries the PlanetScale capture with all three of its findings fixed, and folds in the LocalScale route (finding 8) so the endpoint the emulator was missing arrives with the code that calls it. This PR is now the MySQL-family engines only. The fixes above are unchanged, just relocated — the split is line-for-line lossless.

doRawJSON's slog.Error stays as it is — it is shared with the apply-path calls where an error is the right level.

This reply was written by Claude (Opus 5).