docs: document the FK + covering-index operational rule#35
Conversation
Promote the FK + implicit-covering-index issue from "not yet implemented" to a deliberate operational rule. myschema will not auto-detect or auto-skip the implicit index that MySQL creates when you declare a FK on un-indexed columns; the desired SQL must spell out the matching `KEY` itself. Rationale: a declarative tool works best when "what you wrote" is what gets applied. An implicit-index suppression rule (mirroring MySQL's reuse logic) would silently rewrite the user's intent. Requiring the explicit `KEY` keeps desired SQL self-documenting and matches what `dump` already emits, so `dump → apply` round-trips unaffected. Changes: - New `CAVEATS.md` with the full rule, examples (good vs. bad), the reason myschema doesn't infer it, and the migration-from-existing- schema escape hatch (`dump` first). - `AGENTS.md`: drop the long "Not yet implemented" paragraph; replace with a short "Operational rules" pointer that references CAVEATS.md. - `TODO.md`: remove the "FK-implicit covering indexes look like drift" entry — no longer a planned fix. 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 #35 +/- ##
=======================================
Coverage 73.40% 73.40%
=======================================
Files 27 27
Lines 2151 2151
=======================================
Hits 1579 1579
Misses 412 412
Partials 160 160 ☔ 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 reclassifies myschema’s foreign-key implicit-index behavior from a planned fix to a documented operational rule, making the expectation explicit in repository docs instead of TODOs. It fits the codebase’s declarative-schema model by documenting that desired SQL must spell out FK covering indexes rather than relying on MySQL’s implicit index creation.
Changes:
- Add a new
CAVEATS.mddocument describing the FK covering-index rule, rationale, examples, and migration guidance. - Update
AGENTS.mdto point readers atCAVEATS.mdand summarize the rule under operational constraints. - Remove the old FK-implicit-index item from
TODO.mdnow that it is no longer considered a planned implementation gap.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
TODO.md |
Removes the FK implicit-index item from the outstanding work list. |
CAVEATS.md |
Adds the canonical user-facing explanation of the FK covering-index rule. |
AGENTS.md |
Replaces the old limitation note with a short operational-rule summary and pointer to CAVEATS.md. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Four nits, all valid: - testdata/plan/fk_implicit_index_drift.yml: header was telling maintainers to delete the fixture once the AGENTS.md "Not yet implemented" note disappeared — but this PR removed that note. Re- anchor the fixture as a permanent pin of the CAVEATS.md rule. - AGENTS.md operational-rules summary said "explicit matching `KEY`", which narrows the actual rule (PRIMARY KEY / UNIQUE also count). Widen to "PRIMARY KEY / UNIQUE / KEY" so readers don't add a redundant secondary index when a PK already covers the FK. - AGENTS.md summary also lost the operational nuance: an FK without covering index does not always make apply error — it surfaces as a `-- skipped: DROP INDEX` in plan by default, and only errors with 1553 if the user passes `--allow-drop=index`. Restore that detail. - CAVEATS.md "Requiring an explicit `KEY`" had the same narrowing. Widen to "explicit covering index (PRIMARY KEY / UNIQUE / KEY)". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CAVEATS.md "The rule only matters when you handwrite a CREATE TABLE from scratch" was too narrow. myschema also accepts standalone `ALTER TABLE … ADD CONSTRAINT … FOREIGN KEY` in desired SQL (it's listed in AGENTS.md "In scope (v1)"), and the same implicit-index drift fires there too. Widen the wording so both shapes are covered. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 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.
Summary
Promote the "FK-implicit covering indexes look like drift" item from
TODO.md/ "Not yet implemented" to a deliberate operational rule: myschema will not auto-detect or auto-skip the implicit index MySQL creates for un-indexed FK columns. Desired SQL must spell out the matchingKEYitself.Why this is a rule, not a missing feature
A declarative tool works best when "what you wrote" is what gets applied. An implicit-index suppression heuristic (mirroring MySQL's reuse logic) would silently rewrite the user's intent and make the desired SQL non-self-describing. Requiring the explicit
KEYis the same shapedumpalready emits, sodump → applyround-trips unaffected.Changes
CAVEATS.md(new) — full rule with good / bad examples, the rationale, and the migration-from-existing-schema escape hatch (dumpfirst).AGENTS.md— drop the long "Not yet implemented" paragraph; replace with a short "Operational rules" pointer that referencesCAVEATS.md. Future operational rules go in the same place.TODO.md— remove the "FK-implicit covering indexes look like drift" entry; no longer a planned fix.No code changes.
Test plan
make lint(no Go changes; just confirming nothing else regressed)🤖 Generated with Claude Code