fix(mysql): stop the schema diff from reporting permanent drift on foreign keys - #445
Merged
Merged
Conversation
…reign keys
A MySQL table that declares a foreign key reported drift on *every* schema
check, and the migration it generated could not run. Reported downstream as
wolverine#3983, where every Wolverine node logged an error on every startup:
DROP INDEX `fk_wolverine_node_assignments_node_id` ON <schema>.wolverine_node_assignments
MySqlException: Cannot drop index '...': needed in a foreign key constraint
Three separate defects stacked up here.
1. MySQL implicitly creates a backing index for every FOREIGN KEY constraint,
normally named after the constraint, and information_schema.STATISTICS
reports it like any other index. A table that declared only the constraint
therefore diffed as having one extra index, and InnoDB refuses the DROP
INDEX that follows (error 1553). An index that only exists to back a
surviving foreign key is now kept out of the comparison. Indexes the
expected table declares by name are still compared, so real drift on a
deliberately declared index is unaffected, and the check matches on the
index's leading columns rather than its name — MySQL reuses an existing
covering index instead of creating its own when it can.
2. WriteUpdate emitted index changes before foreign key changes, so dropping a
foreign key also failed: the DROP INDEX for its backing index went out
first. Constraints now come off first, then indexes are reshaped, then the
constraints go back on — which is also the order ADD CONSTRAINT needs, since
MySQL will not add a foreign key that has no covering index.
3. Table(DbObjectName) kept whatever identifier it was handed. MySqlObjectName
renders `schema`.`name` and a plain DbObjectName renders schema.name; since
DbObjectName equality compares exactly those strings, a hand-built
identifier never compared equal to the same table read back out of the
catalog, and the foreign key reported as Different forever. The identifier
and a foreign key's LinkedTable are both normalized now, which also fixes
DDL for any identifier that actually needs escaping.
The first migration against an empty database is a pure CREATE, so none of
this was reachable until the second run — which is why it reproduced against a
brand-new database and then repeated on every start.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxbBpN6PmRB5CMSSTdGNr3
The first pass at this held back every index whose leading columns matched a surviving foreign key. That is more than InnoDB asks for: it requires that *some* index cover the constrained columns, not that any particular one survive. Protecting all of them meant a redundant index -- a narrow one the caller has stopped declaring in favour of a wider one, say -- could never be dropped again. Each surviving constraint is now checked for a cover that will still be there when the DROP INDEX statements run: the primary key when it is not being rebuilt, an index the expected table declares and the database already has unchanged, or an index already held back for another constraint. Only when none of those hold is one index kept, preferring the constraint-named one and then the narrowest. Everything else is an ordinary extra again. The primary key comparison moves ahead of the index comparison in compare(), because the index rule now depends on whether the key is stable, and covers() is split out of backs() so index columns and primary key columns run through the same prefix test. Emission order is deliberately unchanged. Creating the new indexes before dropping the old ones would let a not-yet-created declared index count as a cover, but InnoDB retires its own backing index the moment another index can serve the constraint, so the DROP INDEX that followed hit a key that was already gone. Leaving the order alone costs nothing: that case still settles in a single pass, because MySQL does the cleanup itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Many thanks @jeremydmiller - when I can expect this fix to make its way into a new release of Wolverine? |
This was referenced Aug 20, 2026
Member
Author
|
@cyclr-adrian Maybe today? That's setting yourself up for the sales pass to always have your stuff at the top of the priority queue: https://jasperfx.net/support-plans/ ;-) |
This was referenced Aug 20, 2026
Merged
Merged
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.
Fixes the upstream half of wolverine#3983, where every Wolverine node on MySQL logged an error on every startup:
The reporter had it right that the index and the constraint are the same object. Digging in, there turned out to be three independent defects, all in
Weasel.MySql, and any MySQL table that declares a foreign key hits them. The first migration against an empty database is a pureCREATE, so none of it is reachable until the second run — which is why it reproduces against a brand-new database and then repeats forever.Credit to @cyclr-adrian, who hit the same failure and diagnosed defect 1 independently in #444 — including the detail that MySQL only names the index after the constraint when it creates that index itself. This PR supersedes that one, and takes defects 2 and 3 with it.
1. A foreign key's backing index was diffed as an extra index
MySQL implicitly creates a backing index for every
FOREIGN KEYconstraint, normally named after the constraint, andinformation_schema.STATISTICSreports it like any other index.Table.FetchExistingread it back,TableDeltafound no counterpart in the expected table, and emitted aDROP INDEXthat InnoDB refuses (error 1553).An index that exists only to back a surviving foreign key is now kept out of the comparison. Three details worth calling out:
DROP INDEXstatements run: the primary key when it is not being rebuilt, an index the expected table declares that the database already has unchanged, or an index already held back for another constraint. Only when none of those hold is one index kept, preferring the constraint-named one and then the narrowest. Everything else is an ordinary extra again.An index that is about to be created does not count as a cover, because it is created after the drops. That costs nothing in the case it looks like it should — a declared index replacing the implicit one — because InnoDB retires its own backing index by itself the moment another index can serve the constraint. The
CREATE INDEXalone settles it and the next comparison sees no extra at all, so that case is still a single pass. (Creating the new indexes before dropping the old ones, to widen what counts as a cover, is exactly what does not work: theDROP INDEXthat follows hits a key MySQL has already removed.)2.
WriteUpdateemitted index changes before foreign key changesSo removing a foreign key failed for the same reason: the
DROP INDEXfor its backing index went out ahead of theDROP FOREIGN KEY. Constraints now come off first, then indexes are reshaped, then the constraints go back on — which is also the orderADD CONSTRAINTneeds, since MySQL will not add a foreign key that has no covering index.3.
Table(DbObjectName)did not normalize its identifierMySqlObjectNamerenders`schema`.`name`and a plainDbObjectNamerendersschema.name.DbObjectNameequality compares exactly those strings, so a hand-built identifier never compared equal to the same table read back out of the catalog, and the foreign key reported asDifferenton every check — aDROP FOREIGN KEY+ re-ADDon every migration.The identifier and a foreign key's
LinkedTableare both normalized now. This also fixes DDL for any identifier that actually needs escaping — the unquoted form was being emitted intoREFERENCESclauses.Only MySQL is affected by #3: PostgreSQL, SQL Server and Oracle all render qualified names unquoted, so their plain and provider-specific object names already compare equal.
Testing
Two new test classes, both of which fail on
master:foreign_key_backing_indexes— round-trip with no delta; apply-twice is a no-op; dropping a foreign key also clears its backing index; an unrelated index is still reported as extra; a pre-existing index backing a constraint under a different name is protected.table_identifier_normalization— identifier andLinkedTablenormalization, and a no-drift round trip for a foreign key declared with a plainDbObjectName.Four more in
foreign_key_backing_indexescover the last-cover rule specifically — a redundant index is dropped once a declared index covers the constraint; the primary key backing a constraint leaves nothing to protect; exactly one index is kept when two cover the same constraint and neither is declared; and, as a guard, declaring an index over the implicit one still settles in a single pass. The first three fail against the earlier protect-everything rule.Weasel.MySql.Testsis green at 246 tests againstmysql:8.0, and the fullWeasel.slnxbuilds clean (0 errors).Verified end-to-end against Wolverine with a local project reference: with this change plus JasperFx/wolverine#3984,
MigrateAsync()three times over followed byAssertDatabaseMatchesConfigurationAsync()passes, where today it reports four separate drift items.🤖 Generated with Claude Code
https://claude.ai/code/session_01HxbBpN6PmRB5CMSSTdGNr3