refactor(tern): read table catalog and size estimates in one scan

[aparajon]

A DETAILED schema pull scanned information_schema.tables twice per namespace: once for each table's kind and comment, once for its row-count and on-disk-size estimates. Both applied the same WHERE table_schema = ? and ORDER BY table_name and were keyed by table_name over the same rows. This folds them into one query that populates the catalog fields and the estimate fields in a single pass — one traversal of the view and one round trip instead of two.

  before                        after
  ──────────────────────        ──────────────────────
  table kinds + comments        table kinds + comments
  columns                         + row/size estimates
  indexes                       columns
  foreign keys                  indexes
  row/size estimates            foreign keys
  ──────────────────────        ──────────────────────
  5 information_schema scans    4 information_schema scans

Measured on a synthetic 2000-table schema, MySQL 8.0.46:

query shape	cached statistics	after statistics expiry
kind + comment only	4.7 ms	4.2 ms
estimates only	6.1 ms	12.7 ms
merged (this PR)	7.2 ms	14.5 ms
before, both scans	10.8 ms	16.9 ms

The estimate columns are the pricier half, and they roughly double once the data dictionary's cached statistics have expired. information_schema_stats_expiry is what governs that — not innodb_stats_on_metadata, which defaults to OFF on 8.0 and applies only when innodb_stats_persistent is also OFF. Kind and comment are cheap and flat across both columns of the table. So the win here is dropping a redundant traversal, not halving statistics retrieval.

Worth keeping the scale honest: this trims a slice of the DETAILED add-on, which is itself the minority of pull cost. On the same schema the four DETAILED scans total roughly 54 ms — columns at 24.6 ms and statistics at 19.1 ms dominate it, not tables — against roughly 424 ms that every pull already pays for the per-table SHOW CREATE TABLE loop.

Ordering was safe to collapse: the estimate load previously ran last, after columns, indexes and foreign keys, but none of those read the estimate fields — they only append to Columns / Indexes / ForeignKeys. ensurePulledTableCatalog semantics are unchanged, since the table load runs first and its direct assignment is what creates every entry; the estimates loop's ensurePulledTableCatalog call could never reach its create branch for a table already in pulledTables. The guard that populates only tables present in pulledTables is untouched.

---

This PR was created by Claude Code (claude-opus-5).

[Copilot]

🟢 Approval recommended

The refactor is tightly scoped to DETAILED catalog pulls, removes redundant work without changing observable catalog detail gating or table filtering semantics, and keeps error handling intact.

Pull request overview

This PR reduces the cost of DETAILED catalog pulls by collapsing two per-namespace scans of information_schema.tables into a single query that fetches both table metadata (kind/comment) and engine estimates (row count + size) in one pass. This targets the dominant pull-time expense on MySQL/InnoDB systems where reading table_rows / data_length / index_length can be particularly costly.

Changes:

• Folded table kind/comment loading and row/size estimate loading into a single information_schema.tables query in loadTableCatalog.
• Removed the now-redundant loadTableEstimates pass and its call site, preserving the existing “DETAILED-only” catalog behavior.

File summaries

File	Description
pkg/tern/local_client.go	Merges table catalog + estimate loading into one scan of information_schema.tables, removing the second scan to cut pull-time cost.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 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).

Clean consolidation — +18/-41 in one file, one commit, all 38 checks green. Two queries with an identical WHERE table_schema = ? and ORDER BY table_name over information_schema.tables, collapsed into one scan.

The regression I went looking for isn't there. The removed loadTableEstimates carried a docstring saying its estimates "are only loaded at DETAILED catalog detail," so the obvious way for this merge to go wrong is by welding a conditional expensive read onto an unconditional cheap one — quietly making every catalog pull pay for table_rows / data_length / index_length. That's not what happens: pullNamespaceCatalog early-returns at :1046 unless catalogDetail == PULL_CATALOG_DETAIL_DETAILED, so loadTableCatalog and loadTableEstimates already had exactly the same reachability. The docstring was describing the caller's gate, not a gate of its own. Merging is a strict reduction: one scan where there were two, at the same detail level.

The rest of the equivalence holds up:

• Both queries filtered identically and both applied the same if _, ok := pulledTables[tableName]; ok guard, so the set of catalog entries is unchanged. The old code's ensurePulledTableCatalog in the estimates pass could never actually create an entry — the first pass had already created one for every row the second pass could see.
• DataSizeBytes: dataLength.Int64 + indexLength.Int64 is the same expression, with the same sql.NullInt64 NULL→0 behavior, so views still land on zeros rather than erroring on the scan.
• ensurePulledTableCatalog isn't orphaned — the column, index and foreign-key loaders still use it (:1138, :1194, :1248).
• Only one information_schema.tables query remains in the file, so there's no leftover second scan.

One finding, in the comment rather than the code: the cost note attributes the expense to innodb_stats_on_metadata, which is the MySQL 5.6-era lever. On 8.0 it defaults to OFF, and the variable that actually governs whether these columns are served from cache or force the server to open each table is information_schema_stats_expiry (default 86400s). So on this fleet the real cost profile is "cheap while the cached statistics are fresh, expensive on the first pull after they expire" — which is a materially different operational story from the one the comment tells, and the same knob I flagged on #1238's plan-time probe. Worth correcting so a future reader doesn't go looking at the wrong variable when a DETAILED pull is slow.

Minor design note, not a request: kind/comment and the size estimates are now welded into a single statement. That's the right trade at one round trip versus two, but it does mean a future catalog detail level between BASIC and DETAILED — structure without the expensive statistics columns — would have to un-merge this rather than just skip a call. Cheap to reverse if that ever comes up; noting it only because the separation used to buy that for free.

Not blocking.

[aparajon]

🤖 Addressing morgo's review.

Correct on innodb_stats_on_metadata, and it's worse than a stale reference — on 8.0 it's OFF by default and only applies when innodb_stats_persistent is OFF, which also defaults the other way, so the comment pointed at a doubly inert variable. Fixed in 011021a to name information_schema_stats_expiry and describe the profile it actually produces.

Chasing it down turned up a bigger error of mine in the summary. I measured the three query shapes on a 2000-table schema (8.0.46): kind+comment 4.7 ms, estimates 6.1 ms, merged 7.2 ms — and kind+comment stays flat at 4.2 ms with information_schema_stats_expiry=0 while estimates roughly double to 12.7 ms. The cheap half never touched statistics, so "doubles that cost" and "halving the dominant cost" were both wrong; the saving is a redundant traversal, ~10.8 ms → 7.2 ms. Also dropped the "most expensive query a pull issues" line: columns (24.6 ms) and statistics (19.1 ms) each cost more than tables (8.9 ms), and the whole DETAILED add-on (~54 ms) is small against the ~424 ms of per-table SHOW CREATE TABLE that every pull already pays. Summary updated with the numbers.

Taking the design note as noted, not actioned — at DETAILED both halves are always wanted, and un-merging is a one-function change if an intermediate detail level ever needs structure without statistics.

This reply was written by Claude Code (claude-opus-5).