catalog: composed-state matrix tests for loadColumns / loadIndexes#86
Merged
Conversation
First item from the PR #84 postmortem audit: combinations of attributes that share parser state in the catalog reader. Existing tests covered each token in isolation; the PR #84 INVISIBLE × ON UPDATE bug showed the failure mode where one branch's TrimPrefix captures another branch's trailing token. Pin the remaining combinations so the same bug class can't slip in for other compositions. TestColumnAttributeMatrix: - AUTO_INCREMENT × INVISIBLE — both EXTRA tokens on one row - STORED GENERATED × INVISIBLE — INVISIBLE strip must leave the STORED-GENERATED substring intact - VIRTUAL GENERATED × COMMENT — generation expression survives alongside a per-column COMMENT TestIndexAttributeMatrix: - UNIQUE × INVISIBLE × COMMENT — three index-level flags on one index - FULLTEXT × COMMENT — INDEX_TYPE classification + COMMENT - SPATIAL × prefix length — SPATIAL classification (prefix-length on parts is a separate test below since SPATIAL rejects SUB_PART) - prefix length × INVISIBLE — per-part SUB_PART + per-index IS_VISIBLE both round-trip - INVISIBLE on multi-column key — multi-row accumulation with one INVISIBLE flag agreed across rows All eight sub-tests against MySQL 8.0 round-trip clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #86 +/- ##
==========================================
+ Coverage 86.66% 86.84% +0.18%
==========================================
Files 30 30
Lines 3277 3277
==========================================
+ Hits 2840 2846 +6
+ Misses 269 264 -5
+ Partials 168 167 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds catalog-package “matrix” tests to cover combinations of column/index metadata attributes that share parsing state in catalog.loadColumns / catalog.loadIndexes, motivated by the PR #84 postmortem (composed-token parsing regressions).
Changes:
- Add
TestColumnAttributeMatrixto exercise composedinformation_schema.COLUMNS.EXTRAtoken combinations (e.g.,AUTO_INCREMENT+INVISIBLE, generated columns +INVISIBLE, generated +COMMENT). - Add
TestIndexAttributeMatrixto exercise composedinformation_schema.STATISTICSmetadata combinations (e.g., UNIQUE + INVISIBLE + COMMENT, FULLTEXT + COMMENT, prefix length + INVISIBLE, multi-column INVISIBLE).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Nine review nits: - Eight `tbl, _ := tables.GetOk(...)` sites in the new TestColumnAttributeMatrix / TestIndexAttributeMatrix sub-tests dropped the `ok` return, so a missing table would surface as a nil-pointer panic on the next dereference instead of a clear test failure. Switch every site to `tbl, ok := tables.GetOk(...); require.True(t, ok)` to match the convention already used by TestTablesRoundTrip and the older catalog tests. - The "SPATIAL × prefix length" sub-test name overstated what the body covers — MySQL rejects prefix-length on SPATIAL indexes, so the test only pins SPATIAL classification (the prefix-length aspect lives in the dedicated "prefix length × INVISIBLE" sub-test on a regular b-tree index). Rename to "SPATIAL classification" and update the comment to call out where the prefix-length pin actually lives. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 task
winebarrel
added a commit
that referenced
this pull request
May 6, 2026
Five of the six items from the PR #84 postmortem audit landed: - composed-state catalog tests → PR #86 - kitchen-sink round-trip → PR #87 - emitter ordering pins → PR #88 - diff cross-cutting → PR #89 - directive composition → PR #90 Drop the closed items. Keep `parseCreateTable` column-loop interactions under "Medium — test coverage gaps" — that one was scoped out of the kitchen-sink because the open shapes (inline `UNIQUE × DEFAULT`, inline `REFERENCES × NOT NULL × ON UPDATE`, inline `PK × column-level CHECK`) need direct parser unit tests, not a round-trip fixture. The kitchen-sink uses `email VARCHAR NOT NULL UNIQUE` (no DEFAULT), an explicit named CONSTRAINT FK (not inline REFERENCES), and a table-level CHECK (not the column-level shape that exercises the vitess promotion path), so the gap is real. The "Medium — silent diffs / fidelity gaps" section keeps the real open items, including the three new ones surfaced by the audit itself (REFERENCES auto-name, vitess-keyword identifier drift, CHECK referencing renamed column). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First item from the PR #84 postmortem audit (TODO.md "Medium — test coverage audit", item 1). Combinations of attributes that share parser state in the catalog reader. Existing tests covered each token in isolation; the PR #84 INVISIBLE × ON UPDATE bug showed the failure mode where one branch's TrimPrefix captures another branch's trailing token. Pin the remaining combinations so the same bug class can't slip in for other compositions.
TestColumnAttributeMatrix:AUTO_INCREMENT × INVISIBLE— both EXTRA tokens on one rowSTORED GENERATED × INVISIBLE— INVISIBLE strip must leave theSTORED GENERATEDsubstring intact for the Stored detectionVIRTUAL GENERATED × COMMENT— generation expression survives alongside a per-columnCOMMENTTestIndexAttributeMatrix:UNIQUE × INVISIBLE × COMMENT— three index-level flags on one indexFULLTEXT × COMMENT— INDEX_TYPE classification + COMMENTSPATIAL × prefix length— SPATIAL classification (note: prefix length on parts is a separate sub-test since SPATIAL rejects SUB_PART)prefix length × INVISIBLE— per-part SUB_PART + per-index IS_VISIBLE both round-tripINVISIBLE on multi-column key— multi-row accumulation with one INVISIBLE flag agreed across rowsTest plan
make lintclean.make testall six packages green.🤖 Generated with Claude Code