Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 67 additions & 1 deletion lib/Maho/Db/Schema/Canonicalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

use Doctrine\DBAL\Schema\DefaultExpression\CurrentTimestamp;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\Index\IndexType;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Types\FloatType;
use Doctrine\DBAL\Types\SmallFloatType;
Expand Down Expand Up @@ -38,7 +39,9 @@
* - index names: introspected names vs DBAL's autogenerated ones;
* - phantom FK indexes: introspection synthesizes a single-column index for a
* foreign key backed only by a composite prefix, with no physical
* counterpart (see stripPhantomIndexes()).
* counterpart (see stripPhantomIndexes());
* - index column names: introspection marks them quoted, the declarative
* schema leaves them bare (see unquoteIndexColumns()).
*
* The equivalence applied here is purely representational, so it never hides a
* genuine structural difference. Methods mutate the passed tables in place;
Expand All @@ -56,6 +59,7 @@ final class Canonicalizer
public static function reconcile(Table $live, Table $target, array $physicalLiveIndexNames): void
{
self::stripPhantomIndexes($live, $physicalLiveIndexNames);
self::unquoteIndexColumns($live);
self::alignIndexNames($live, $target);
self::alignTableCharset($live, $target);
self::stripColumnComments($live);
Expand Down Expand Up @@ -103,6 +107,68 @@ private static function stripPhantomIndexes(Table $table, array $physicalIndexNa
}
}

/**
* Re-express every live index with bare column names.
*
* 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 target.
* The Comparator sees through that, but AbstractMySQLPlatform does not: it
* folds an index drop and an index creation over the same columns into one
* `ALTER TABLE ... DROP INDEX a, ADD INDEX b (...)` only when the two raw
* column lists match, read through Index::getColumns(), the same legacy
* accessor that leaked introspection quoting in doctrine/dbal#7392. Unfolded,
* the drop runs on its own, and MySQL refuses to drop the index backing a
* foreign key: "needed in a foreign key constraint" (error 1553, which
* FOREIGN_KEY_CHECKS=0 does not lift). That is every legacy install whose
* composite UNIQUE key the declarative target re-declares as a plain index.
*
* Each index keeps its own name, spelled exactly as introspected: that
* quoting is load-bearing on Postgres, where a legacy index named as a bare
* hex hash may start with a digit and is only valid quoted.
*
* @todo Drop this method (and its call in reconcile()) once
* https://github.com/doctrine/dbal/pull/7475 (the upstream fix, which
* compares the same column lists through Index::getIndexedColumns())
* is released and composer.json requires a version carrying it. The
* fold then matches on unquoted values on its own, and the quoting
* becomes representation the Comparator already sees through.
*/
private static function unquoteIndexColumns(Table $table): void
{
foreach ($table->getIndexes() as $index) {
if (self::isPrimaryIndex($table, $index)) {
continue;
}

$columns = [];
$lengths = [];
$quoted = false;
foreach ($index->getIndexedColumns() as $indexedColumn) {
$identifier = $indexedColumn->getColumnName()->getIdentifier();
$columns[] = $identifier->getValue();
$lengths[] = $indexedColumn->getLength();
$quoted = $quoted || $identifier->isQuoted();
}
if (!$quoted) {
continue;
}

$name = $index->getObjectName()->toString();
$type = $index->getType();
$table->dropIndex($name);
if ($type === IndexType::UNIQUE) {
$table->addUniqueIndex($columns, $name, ['lengths' => $lengths]);
} else {
$flags = match ($type) {
IndexType::FULLTEXT => ['fulltext'],
IndexType::SPATIAL => ['spatial'],
default => [],
};
$table->addIndex($columns, $name, $flags, ['lengths' => $lengths]);
}
}
}

/**
* Rename each live index that matches a target index by structure to the
* target's name, so the Comparator treats the pair as identical and emits
Expand Down
29 changes: 29 additions & 0 deletions tests/Backend/Unit/Maho/Db/Schema/CanonicalizerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,35 @@ function canonTable(string $name): Table
expect($live->hasIndex('LEGACY_HASH_NAME'))->toBeFalse();
});

it('re-expresses quoted live index columns as bare names', function () {
// What introspection produces: index name and column names marked quoted.
$live = canonTable('t');
$live->addColumn('customer_id', Types::INTEGER, ['unsigned' => true]);
$live->addColumn('product_id', Types::INTEGER, ['unsigned' => true]);
$live->addUniqueIndex(['"customer_id"', '"product_id"'], '"UNQ_LEGACY"');

$target = canonTable('t');
$target->addColumn('customer_id', Types::INTEGER, ['unsigned' => true]);
$target->addColumn('product_id', Types::INTEGER, ['unsigned' => true]);
$target->addIndex(['customer_id', 'product_id'], 'IDX_TARGET');

Canonicalizer::reconcile($live, $target, ['"UNQ_LEGACY"']);

// The bare column names let AbstractMySQLPlatform match this drop against
// the target's creation and fold both into a single ALTER TABLE.
$index = $live->getIndex('UNQ_LEGACY');
$columns = array_map(
static fn($indexedColumn): string => $indexedColumn->getColumnName()->getIdentifier()->getValue(),
$index->getIndexedColumns(),
);
expect($columns)->toBe(['customer_id', 'product_id']);
foreach ($index->getIndexedColumns() as $indexedColumn) {
expect($indexedColumn->getColumnName()->getIdentifier()->isQuoted())->toBeFalse();
}
// Uniqueness survives the rewrite: it is what makes this a real change.
expect($index->getType())->toBe(Doctrine\DBAL\Schema\Index\IndexType::UNIQUE);
});

it('drops a phantom index that has no physical counterpart', function () {
$live = canonTable('t');
$live->addColumn('a', Types::INTEGER, ['unsigned' => true]);
Expand Down
Loading