test: diff cross-cutting / order-sensitivity pins#89
Merged
Conversation
Fourth item from the PR #84 postmortem audit. Mutators in the diff layer run in sequence on the same `current` map (table renames → column renames → cross-table FK ref rewrites → diffTable). Existing rename_test.go covers 1-to-1 cases; pin three open shapes where multiple kinds of change interact. - error_column_rename_destination_exists.yml: catalog has both `old_name` and `name`; desired says "renamed-from old_name → name" but `name` already exists. The guard in applyColumnRenames refuses with "destination already exists" rather than silently clobbering one of the two columns. - index_rename_with_unrelated_drop.yml: rename one index AND drop a different index in the same plan. Both pass through the `Stmts` bucket; the diff orders them rename-first / drop-after so the resulting plan is two clean ALTER TABLEs that compose without interfering. Pins per-index pass-order doesn't break. - bulk_alter_partition_with_charset_and_column.yml: REORGANIZE PARTITION must stay its own ALTER under --bulk-alter (partition ops excluded per CAVEATS) while DEFAULT CHARSET + ADD COLUMN fold together. Three change kinds in one diff covering the bulk-alter excluded-set / combinable-set boundary. 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 #89 +/- ##
==========================================
+ Coverage 86.66% 86.72% +0.06%
==========================================
Files 30 30
Lines 3277 3277
==========================================
+ Hits 2840 2842 +2
+ Misses 269 268 -1
+ 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
This PR adds new YAML apply fixtures to pin order-sensitive and cross-cutting behaviors in the diff/apply pipeline, expanding regression coverage for multi-mutator interactions identified in the PR #84 postmortem audit.
Changes:
- Add a fixture that asserts an error is raised when a
myschema:renamed-fromcolumn rename would collide with an existing destination column. - Add a fixture that pins statement ordering when an index rename and an unrelated index drop occur in the same plan.
- Add a fixture that pins
--bulk-alterbehavior at the boundary between non-combinable partition operations and combinable ALTER specs (DEFAULT CHARSET + ADD COLUMN).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| testdata/apply/error_column_rename_destination_exists.yml | Pins the rename-collision error path for column renames using error: substring matching. |
| testdata/apply/index_rename_with_unrelated_drop.yml | Pins rename-before-drop ordering for independent index changes in a single plan. |
| testdata/apply/bulk_alter_partition_with_charset_and_column.yml | Pins that partition ops stay separate under --bulk-alter, while charset + column add still combine. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Fourth item from the PR #84 postmortem audit (TODO.md "Medium — test coverage audit", item 4). Mutators in the diff layer run in sequence on the same
currentmap (table renames → column renames → cross-table FK ref rewrites →diffTable). Existingrename_test.gocovers 1-to-1 cases; pin three open shapes where multiple kinds of change interact.error_column_rename_destination_exists.yml— catalog has bothold_nameandname; desired says "renamed-fromold_name→name" butnamealready exists. The guard inapplyColumnRenamesrefuses withdestination already existsrather than silently clobbering one of the two columns. Pinned via theerror:field of the YAML harness.index_rename_with_unrelated_drop.yml— rename one index AND drop a different index in the same plan. Both pass through theStmtsbucket; the diff orders them rename-first / drop-after so the resulting plan is two cleanALTER TABLEs that compose without interfering.bulk_alter_partition_with_charset_and_column.yml—REORGANIZE PARTITIONmust stay its own ALTER under--bulk-alter(partition ops excluded per CAVEATS) whileDEFAULT CHARSET+ADD COLUMNfold together. Three change kinds in one diff covering the bulk-alter excluded-set / combinable-set boundary.verify_no_drift: falsematcheschange_table_charset.yml's baseline (DEFAULT CHARSET + string columns converges in two applies).Test plan
make lintclean.make testall six packages green.🤖 Generated with Claude Code