Visitar URL original
[ticket/17746] Keep indexes that are dropped and added in one schema … · phpbb/phpbb@4e8e6ef · GitHub
Skip to content

Commit 4e8e6ef

Browse files
boneliferbonelifer
committed
[ticket/17746] Keep indexes that are dropped and added in one schema change
Doctrine reports an index dropped and added again with the same name but other columns as a modified index. The comparator rebuilt the table diff from the added and dropped indexes only, so the change was lost and the old index stayed. Add each modified index as dropped and added. When a migration drops an index by its unprefixed key name and the table also has the generated, prefixed name, drop the generated index. PHPBB-17746
1 parent 8f67684 commit 4e8e6ef

4 files changed

Lines changed: 111 additions & 0 deletions

File tree

‎phpBB/phpbb/db/doctrine/comparator.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ public function compareTables(Table $oldTable, Table $newTable): TableDiff
4040
$diff = parent::compareTables($oldTable, $newTable);
4141
$added_indexes = $diff->getAddedIndexes();
4242
$dropped_indexes = $diff->getDroppedIndexes();
43+
44+
// Preserve changed index definitions when rebuilding the table diff.
45+
foreach ($diff->getModifiedIndexes() as $index)
46+
{
47+
$dropped_indexes[] = $oldTable->getIndex($index->getName());
48+
$added_indexes[] = $index;
49+
}
4350
$added_index_names = array_map([$this, 'get_index_name'], $added_indexes);
4451
$dropped_index_names = array_map([$this, 'get_index_name'], $dropped_indexes);
4552

‎phpBB/phpbb/db/tools/doctrine.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -988,6 +988,14 @@ protected function schema_index_drop(Schema $schema, string $table_name, string
988988
$table = $schema->getTable($table_name);
989989
$short_table_name = table_helper::generate_shortname(self::remove_prefix($table_name, $this->table_prefix));
990990

991+
// Legacy migrations pass the unprefixed key name. Prefer that table's
992+
// generated name if an unrelated unprefixed key is also present.
993+
$prefixed_name = self::add_prefix($index_name, $short_table_name);
994+
if (!str_starts_with($index_name, $short_table_name) && $table->hasIndex($prefixed_name))
995+
{
996+
$index_name = $prefixed_name;
997+
}
998+
991999
if (!$table->hasIndex($index_name))
9921000
{
9931001
$index_name = !str_starts_with($index_name, $short_table_name) ? self::add_prefix($index_name, $short_table_name) : self::remove_prefix($index_name, $short_table_name);

‎tests/dbal/db_tools_test.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,28 @@ public function test_perform_schema_changes_drop_columns()
463463
));
464464
}
465465

466+
public function test_perform_schema_changes_replaces_index_with_same_name()
467+
{
468+
// Drop a unique index and add it back with another column, in one schema change
469+
$this->tools->perform_schema_changes([
470+
'drop_keys' => [
471+
'prefix_table_name' => ['i_comp_uniq'],
472+
],
473+
'add_unique_index' => [
474+
'prefix_table_name' => [
475+
'i_comp_uniq' => ['c_vchar_size', 'c_usint', 'c_uint'],
476+
],
477+
],
478+
]);
479+
480+
$index_name = \phpbb\db\doctrine\table_helper::generate_shortname('table_name') . '_i_comp_uniq';
481+
$this->assertTrue($this->tools->sql_unique_index_exists('prefix_table_name', $index_name));
482+
483+
$indexes = $this->doctrine_db->createSchemaManager()->listTableIndexes('prefix_table_name');
484+
$this->assertArrayHasKey($index_name, $indexes);
485+
$this->assertSame(['c_vchar_size', 'c_usint', 'c_uint'], $indexes[$index_name]->getColumns());
486+
}
487+
466488
public function test_index_exists()
467489
{
468490
$this->assertTrue($this->tools->sql_index_exists('prefix_table_name', \phpbb\db\doctrine\table_helper::generate_shortname('table_name') . '_i_simple'));

‎tests/dbal/doctrine_comparator_test.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php
2+
/**
3+
*
4+
* This file is part of the phpBB Forum Software package.
5+
*
6+
* @copyright (c) phpBB Limited <https://www.phpbb.com>
7+
* @license GNU General Public License, version 2 (GPL-2.0)
8+
*
9+
* For full copyright and license information, please see
10+
* the docs/CREDITS.txt file.
11+
*
12+
*/
13+
14+
class phpbb_dbal_doctrine_comparator_test extends phpbb_test_case
15+
{
16+
public static function platforms(): array
17+
{
18+
return [
19+
'MySQL' => [new \Doctrine\DBAL\Platforms\MySQLPlatform()],
20+
'PostgreSQL' => [new \Doctrine\DBAL\Platforms\PostgreSQLPlatform()],
21+
'SQLite' => [new \Doctrine\DBAL\Platforms\SQLitePlatform()],
22+
];
23+
}
24+
25+
/**
26+
* An index dropped and added again with the same name but other columns is a
27+
* "modified" index to Doctrine; the rebuilt diff must still change it.
28+
*
29+
* @dataProvider platforms
30+
*/
31+
public function test_modified_index_is_kept(\Doctrine\DBAL\Platforms\AbstractPlatform $platform): void
32+
{
33+
$old = new \Doctrine\DBAL\Schema\Table('articles');
34+
foreach (['article_id', 'word_id', 'title_match', 'descr_match'] as $column)
35+
{
36+
$old->addColumn($column, 'integer');
37+
}
38+
$old->addUniqueIndex(['article_id', 'word_id', 'title_match'], 'article_match');
39+
40+
$new = clone $old;
41+
$new->dropIndex('article_match');
42+
$new->addUniqueIndex(['article_id', 'word_id', 'title_match', 'descr_match'], 'article_match');
43+
44+
$comparator = new \phpbb\db\doctrine\comparator($platform);
45+
$queries = $platform->getAlterTableSQL($comparator->compareTables($old, $new));
46+
47+
$this->assertNotEmpty($queries);
48+
$this->assertStringContainsString('descr_match', implode(' ', $queries));
49+
}
50+
51+
/**
52+
* A migration that drops an index by its unprefixed key name drops the index
53+
* phpBB generated for it, even when an unprefixed index of that name exists too.
54+
*/
55+
public function test_legacy_key_name_drops_the_generated_index(): void
56+
{
57+
$schema = new \Doctrine\DBAL\Schema\Schema();
58+
$table = $schema->createTable('phpbb_kb_src_wrdmtch');
59+
foreach (['article_id', 'word_id', 'title_match', 'descr_match'] as $column)
60+
{
61+
$table->addColumn($column, 'integer');
62+
}
63+
$table->addUniqueIndex(['article_id', 'word_id', 'title_match'], 'kbsrcwrdmt_un_mtch');
64+
$table->addUniqueIndex(['article_id', 'word_id', 'title_match', 'descr_match'], 'un_mtch');
65+
66+
$reflection = new \ReflectionClass(\phpbb\db\tools\doctrine::class);
67+
$tools = $reflection->newInstanceWithoutConstructor();
68+
$tools->set_table_prefix('phpbb_');
69+
$reflection->getMethod('schema_index_drop')->invoke($tools, $schema, $table->getName(), 'un_mtch', true);
70+
71+
$this->assertFalse($table->hasIndex('kbsrcwrdmt_un_mtch'));
72+
$this->assertTrue($table->hasIndex('un_mtch'));
73+
}
74+
}

0 commit comments

Comments
 (0)