Fixed migration failure when dropping a legacy index that backs a foreign key on MySQL - #1136
Merged
Merged
Conversation
DBAL 4.4 introspection marks an index's column names as quoted, so the
same index reads as ["customer_id"] live and [customer_id] in the
declarative target. AbstractMySQLPlatform combines an index drop and an
index creation over the same columns into a single ALTER TABLE statement,
but it compares the two column lists through the legacy
Index::getColumns() accessor, which returns those raw names. The lists
never match for a live-vs-declarative diff, so the statements are not
combined and the drop is emitted on its own.
That aborts the migration whenever the dropped index is the one covering
a foreign key, which is every legacy install whose composite UNIQUE key
the declarative target re-declares as a plain index:
./maho migrate
SQLSTATE[HY000]: General error: 1553 Cannot drop index
'UNQ_REPORT_COMPARED_PRODUCT_INDEX_CUSTOMER_ID_PRODUCT_ID':
needed in a foreign key constraint
InnoDB refuses the drop because no other index would cover the key, and
disabling FOREIGN_KEY_CHECKS does not lift the restriction, so
startSetup() is no help.
Re-express every live index with bare column names in the Canonicalizer,
alongside the introspection artifacts it already reconciles. The platform
then combines the pair as intended, the cover never lapses, and the
migration converges: the affected table now plans 3 statements instead of
5 and re-plans 0 on a second run.
The upstream fix is doctrine/dbal#7475; the workaround carries a @todo to
drop it once a release requiring it is in place.
|
Successfully created backport PR for |
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.
./maho migrateaborts on a legacy install:Cause
That UNIQUE key is the only index covering
FK ... (customer_id), and InnoDB refuses to drop it. DisablingFOREIGN_KEY_CHECKSdoes not lift the restriction, sostartSetup()is no help; what does lift it is another index taking over the cover in the same statement.DBAL knows this:
AbstractMySQLPlatform::getPreAlterTableIndexForeignKeySQL()combines an index drop and an index creation over the same columns into oneALTER TABLE ... DROP INDEX a, ADD INDEX b (...). It just never fires for us, because it compares the two column lists through the legacyIndex::getColumns()accessor, which returns the raw names: introspection marks them as quoted ("customer_id"), the declarative target keeps them bare (customer_id). Same accessor, same mismatch as #7392 on SQLite. Unfolded, DBAL emits every index drop before every index creation, so the drop of the only covering index runs first:Fix
Canonicalizer::unquoteIndexColumns()re-expresses every live index with bare column names, keeping its name (that quoting is load-bearing on Postgres, where a legacy hex-hash index name can start with a digit and is only valid quoted), its type and its prefix lengths. It sits with the introspection artifacts the class already reconciles, next tostripPhantomIndexes()andalignIndexNames().The platform then combines the pairs as intended, and the same migration plans 3 statements:
Verification
Against a legacy-shaped fixture on MySQL 9.7 (
report_compared_product_indexandcustomer_entityas a Magento install leaves them):plan()returns 0 statements, so the schema convergesCanonicalizerTest, 26 passing,composer lintgreenUpstream
The DBAL side is doctrine/dbal#7475, which fixes the comparison at the source. The workaround carries a
@todoto drop it once a release carrying that fix is required, since the constraint is^4.4today.One shape stays unfixed on DBAL 4.4 either way: an index covering a foreign key that the target drops without a same-column replacement has nothing to combine with, and still fails with 1553. DBAL 5.0 handles it structurally by putting all drops and adds into a single
ALTER TABLE. No table in the current schemas is in that shape.