From d099bb6f9578625bc79f613f65e78ca12b4bbb96 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Wed, 7 Mar 2018 15:25:53 -0500 Subject: [PATCH] Drop the image_comment_temp table It is no longer used. Bug: T188132 Change-Id: Ic8efeddc030f48e82ba861926121b64eca37d169 --- RELEASE-NOTES-1.33 | 3 ++ includes/CommentStore.php | 12 +++-- includes/filerepo/file/LocalFile.php | 52 +++------------------ includes/installer/DatabaseUpdater.php | 23 ++++++--- maintenance/mssql/tables.sql | 14 ------ maintenance/oracle/tables.sql | 9 ---- maintenance/postgres/tables.sql | 7 --- maintenance/tables.sql | 17 ------- tests/parser/ParserTestRunner.php | 1 - tests/phpunit/includes/CommentStoreTest.php | 18 ++----- 10 files changed, 35 insertions(+), 121 deletions(-) diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index 7bbb1656f4..c573a5951c 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -159,6 +159,9 @@ because of Phabricator reports. === Other changes in 1.33 === * (T208871) The hard-coded Google search form on the database error page was removed. +* The image_comment_temp database table, deprecated in 1.32, has been removed. + Since access should be mediated by the CommentStore class, this change + shouldn't affect external code. * … == Compatibility == diff --git a/includes/CommentStore.php b/includes/CommentStore.php index 7a2726f291..cba7a150d3 100644 --- a/includes/CommentStore.php +++ b/includes/CommentStore.php @@ -70,11 +70,7 @@ class CommentStore { 'deprecatedIn' => null, ], 'img_description' => [ - 'table' => 'image_comment_temp', - 'pk' => 'imgcomment_name', - 'field' => 'imgcomment_description_id', - 'joinPK' => 'img_name', - 'stage' => MIGRATION_WRITE_NEW, + 'stage' => MIGRATION_NEW, 'deprecatedIn' => '1.32', ], ]; @@ -226,8 +222,14 @@ class CommentStore { if ( $tempTableStage === MIGRATION_OLD ) { $joinField = "{$alias}.{$t['field']}"; } else { + // Nothing hits this code path for now, but will in the future when we set + // $this->tempTables['rev_comment']['stage'] to MIGRATION_WRITE_NEW while + // merging revision_comment_temp into revision. + // @codeCoverageIgnoreStart $joins[$alias][0] = 'LEFT JOIN'; $joinField = "(CASE WHEN {$key}_id != 0 THEN {$key}_id ELSE {$alias}.{$t['field']} END)"; + throw new LogicException( 'Nothing should reach this code path at this time' ); + // @codeCoverageIgnoreEnd } } else { $joinField = "{$key}_id"; diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index ebbc8f83a9..95ee4f4e56 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -1542,13 +1542,7 @@ class LocalFile extends File { $fields['oi_description'] = 'img_description'; } if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { - $tables[] = 'image_comment_temp'; - $fields['oi_description_id'] = 'CASE WHEN img_description_id = 0 ' - . 'THEN COALESCE(imgcomment_description_id, 0) ELSE img_description_id END'; - $joins['image_comment_temp'] = [ - $wgCommentTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', - [ 'imgcomment_name = img_name' ] - ]; + $fields['oi_description_id'] = 'img_description_id'; } if ( $wgCommentTableSchemaMigrationStage !== MIGRATION_OLD && @@ -1558,16 +1552,13 @@ class LocalFile extends File { // might be missed if a deletion happens while the migration script // is running. $res = $dbw->select( - [ 'image', 'image_comment_temp' ], + [ 'image' ], [ 'img_name', 'img_description' ], [ 'img_name' => $this->getName(), - 'imgcomment_name' => null, 'img_description_id' => 0, ], - __METHOD__, - [], - [ 'image_comment_temp' => [ 'LEFT JOIN', [ 'imgcomment_name = img_name' ] ] ] + __METHOD__ ); foreach ( $res as $row ) { $imgFields = $commentStore->insert( $dbw, 'img_description', $row->img_description ); @@ -1637,10 +1628,6 @@ class LocalFile extends File { [ 'img_name' => $this->getName() ], __METHOD__ ); - if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { - // Clear deprecated table row - $dbw->delete( 'image_comment_temp', [ 'imgcomment_name' => $this->getName() ], __METHOD__ ); - } } $descTitle = $this->getTitle(); @@ -2544,13 +2531,7 @@ class LocalFileDeleteBatch { $fields['fa_description'] = 'img_description'; } if ( $wgCommentTableSchemaMigrationStage >= MIGRATION_WRITE_BOTH ) { - $tables[] = 'image_comment_temp'; - $fields['fa_description_id'] = 'CASE WHEN img_description_id = 0 ' - . 'THEN COALESCE(imgcomment_description_id, 0) ELSE img_description_id END'; - $joins['image_comment_temp'] = [ - $wgCommentTableSchemaMigrationStage === MIGRATION_NEW ? 'JOIN' : 'LEFT JOIN', - [ 'imgcomment_name = img_name' ] - ]; + $fields['fa_description_id'] = 'img_description_id'; } if ( $wgCommentTableSchemaMigrationStage !== MIGRATION_OLD && @@ -2560,16 +2541,13 @@ class LocalFileDeleteBatch { // might be missed if a deletion happens while the migration script // is running. $res = $dbw->select( - [ 'image', 'image_comment_temp' ], + [ 'image' ], [ 'img_name', 'img_description' ], [ 'img_name' => $this->file->getName(), - 'imgcomment_name' => null, 'img_description_id' => 0, ], - __METHOD__, - [], - [ 'image_comment_temp' => [ 'LEFT JOIN', [ 'imgcomment_name = img_name' ] ] ] + __METHOD__ ); foreach ( $res as $row ) { $imgFields = $commentStore->insert( $dbw, 'img_description', $row->img_description ); @@ -2669,8 +2647,6 @@ class LocalFileDeleteBatch { } function doDBDeletes() { - global $wgCommentTableSchemaMigrationStage; - $dbw = $this->file->repo->getMasterDB(); list( $oldRels, $deleteCurrent ) = $this->getOldRels(); @@ -2684,12 +2660,6 @@ class LocalFileDeleteBatch { if ( $deleteCurrent ) { $dbw->delete( 'image', [ 'img_name' => $this->file->getName() ], __METHOD__ ); - if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { - // Clear deprecated table row - $dbw->delete( - 'image_comment_temp', [ 'imgcomment_name' => $this->file->getName() ], __METHOD__ - ); - } } } @@ -3397,8 +3367,6 @@ class LocalFileMoveBatch { * many rows where updated. */ protected function doDBUpdates() { - global $wgCommentTableSchemaMigrationStage; - $dbw = $this->db; // Update current image @@ -3408,14 +3376,6 @@ class LocalFileMoveBatch { [ 'img_name' => $this->oldName ], __METHOD__ ); - if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { - $dbw->update( - 'image_comment_temp', - [ 'imgcomment_name' => $this->newName ], - [ 'imgcomment_name' => $this->oldName ], - __METHOD__ - ); - } // Update old images $dbw->update( diff --git a/includes/installer/DatabaseUpdater.php b/includes/installer/DatabaseUpdater.php index 925fc5a4dc..43000b8b99 100644 --- a/includes/installer/DatabaseUpdater.php +++ b/includes/installer/DatabaseUpdater.php @@ -1282,13 +1282,22 @@ abstract class DatabaseUpdater { */ protected function migrateImageCommentTemp() { global $wgCommentTableSchemaMigrationStage; - if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { - $this->output( "Merging image_comment_temp into the image table\n" ); - $task = $this->maintenance->runChild( - MigrateImageCommentTemp::class, 'migrateImageCommentTemp.php' - ); - $ok = $task->execute(); - $this->output( $ok ? "done.\n" : "errors were encountered.\n" ); + + if ( $this->tableExists( 'image_comment_temp' ) ) { + if ( $wgCommentTableSchemaMigrationStage > MIGRATION_OLD ) { + $this->output( "Merging image_comment_temp into the image table\n" ); + $task = $this->maintenance->runChild( + MigrateImageCommentTemp::class, 'migrateImageCommentTemp.php' + ); + $task->setForce(); + $ok = $task->execute(); + $this->output( $ok ? "done.\n" : "errors were encountered.\n" ); + } else { + $ok = true; + } + if ( $ok ) { + $this->dropTable( 'image_comment_temp' ); + } } } diff --git a/maintenance/mssql/tables.sql b/maintenance/mssql/tables.sql index 63e0aa8890..49115de10a 100644 --- a/maintenance/mssql/tables.sql +++ b/maintenance/mssql/tables.sql @@ -805,20 +805,6 @@ CREATE INDEX /*i*/img_sha1 ON /*_*/image (img_sha1); -- Used to get media of one type CREATE INDEX /*i*/img_media_mime ON /*_*/image (img_media_type,img_major_mime,img_minor_mime); --- --- Temporary table to avoid blocking on an alter of image. --- --- On large wikis like Wikimedia Commons, altering the image table is a --- months-long process. This table is being created to avoid such an alter, and --- will be merged back into image in the future. --- -CREATE TABLE /*_*/image_comment_temp ( - imgcomment_name nvarchar(255) NOT NULL CONSTRAINT FK_imgcomment_name FOREIGN KEY REFERENCES /*_*/image(img_name) ON DELETE CASCADE, - imgcomment_description_id bigint NOT NULL CONSTRAINT FK_imgcomment_description_id FOREIGN KEY REFERENCES /*_*/comment(comment_id), - CONSTRAINT PK_image_comment_temp PRIMARY KEY (imgcomment_name, imgcomment_description_id) -); -CREATE UNIQUE INDEX /*i*/imgcomment_name ON /*_*/image_comment_temp (imgcomment_name); - -- -- Previous revisions of uploaded files. diff --git a/maintenance/oracle/tables.sql b/maintenance/oracle/tables.sql index 6e76851f25..1ccaabf335 100644 --- a/maintenance/oracle/tables.sql +++ b/maintenance/oracle/tables.sql @@ -566,15 +566,6 @@ CREATE INDEX &mw_prefix.image_i03 ON &mw_prefix.image (img_timestamp); CREATE INDEX &mw_prefix.image_i04 ON &mw_prefix.image (img_sha1); CREATE INDEX &mw_prefix.img_actor_timestamp ON &mw_prefix.image (img_actor, img_timestamp); -CREATE TABLE &mw_prefix.image_comment_temp ( - imgcomment_name VARCHAR2(255) NOT NULL, - imgcomment_description_id NUMBER NOT NULL -); -ALTER TABLE &mw_prefix.image_comment_temp ADD CONSTRAINT &mw_prefix.image_comment_temp_pk PRIMARY KEY (imgcomment_name, imgcomment_description_id); -ALTER TABLE &mw_prefix.image_comment_temp ADD CONSTRAINT &mw_prefix.image_comment_temp_fk1 FOREIGN KEY (imgcomment_name) REFERENCES &mw_prefix.image(img_name) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED; -ALTER TABLE &mw_prefix.image_comment_temp ADD CONSTRAINT &mw_prefix.image_comment_temp_fk2 FOREIGN KEY (imgcomment_description_id) REFERENCES &mw_prefix."COMMENT"(comment_id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED; -CREATE UNIQUE INDEX &mw_prefix.imgcomment_name ON &mw_prefix.image_comment_temp (imgcomment_name); - CREATE TABLE &mw_prefix.oldimage ( oi_name VARCHAR2(255) DEFAULT 0 NOT NULL, diff --git a/maintenance/postgres/tables.sql b/maintenance/postgres/tables.sql index 003204dae3..96a061728e 100644 --- a/maintenance/postgres/tables.sql +++ b/maintenance/postgres/tables.sql @@ -463,13 +463,6 @@ CREATE INDEX img_size_idx ON image (img_size); CREATE INDEX img_timestamp_idx ON image (img_timestamp); CREATE INDEX img_sha1 ON image (img_sha1); -CREATE TABLE image_comment_temp ( - imgcomment_name TEXT NOT NULL, - imgcomment_description_id INTEGER NOT NULL, - PRIMARY KEY (imgcomment_name, imgcomment_description_id) -); -CREATE UNIQUE INDEX imgcomment_name ON image_comment_temp (imgcomment_name); - CREATE TABLE oldimage ( oi_name TEXT NOT NULL, oi_archive_name TEXT NOT NULL, diff --git a/maintenance/tables.sql b/maintenance/tables.sql index c46e4c6cd9..3c8b5981b8 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -1214,23 +1214,6 @@ CREATE INDEX /*i*/img_sha1 ON /*_*/image (img_sha1(10)); -- Used to get media of one type CREATE INDEX /*i*/img_media_mime ON /*_*/image (img_media_type,img_major_mime,img_minor_mime); --- --- Temporary table to avoid blocking on an alter of image. --- --- On large wikis like Wikimedia Commons, altering the image table is a --- months-long process. This table is being created to avoid such an alter, and --- will be merged back into image in the future. --- -CREATE TABLE /*_*/image_comment_temp ( - -- Key to img_name (ugh) - imgcomment_name varchar(255) binary NOT NULL, - -- Key to comment_id - imgcomment_description_id bigint unsigned NOT NULL, - PRIMARY KEY (imgcomment_name, imgcomment_description_id) -) /*$wgDBTableOptions*/; --- Ensure uniqueness -CREATE UNIQUE INDEX /*i*/imgcomment_name ON /*_*/image_comment_temp (imgcomment_name); - -- -- Previous revisions of uploaded files. diff --git a/tests/parser/ParserTestRunner.php b/tests/parser/ParserTestRunner.php index 3dee52136d..bc5f1fca3e 100644 --- a/tests/parser/ParserTestRunner.php +++ b/tests/parser/ParserTestRunner.php @@ -1230,7 +1230,6 @@ class ParserTestRunner { // The new tables for comments are in use $tables[] = 'comment'; $tables[] = 'revision_comment_temp'; - $tables[] = 'image_comment_temp'; } if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) { diff --git a/tests/phpunit/includes/CommentStoreTest.php b/tests/phpunit/includes/CommentStoreTest.php index bc1f10140b..4360343981 100644 --- a/tests/phpunit/includes/CommentStoreTest.php +++ b/tests/phpunit/includes/CommentStoreTest.php @@ -117,7 +117,6 @@ class CommentStoreTest extends MediaWikiLangTestCase { MIGRATION_WRITE_BOTH, 'img_description', [ 'img_description_old' => 'img_description', - 'img_description_pk' => 'img_name', 'img_description_id' => 'img_description_id' ], ], @@ -125,14 +124,12 @@ class CommentStoreTest extends MediaWikiLangTestCase { MIGRATION_WRITE_NEW, 'img_description', [ 'img_description_old' => 'img_description', - 'img_description_pk' => 'img_name', 'img_description_id' => 'img_description_id' ], ], 'Image, new' => [ MIGRATION_NEW, 'img_description', [ - 'img_description_pk' => 'img_name', 'img_description_id' => 'img_description_id' ], ], @@ -296,7 +293,6 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'Image, write-both' => [ MIGRATION_WRITE_BOTH, 'img_description', [ 'tables' => [ - 'temp_img_description' => 'image_comment_temp', 'comment_img_description' => 'comment', ], 'fields' => [ @@ -305,10 +301,8 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'img_description_cid' => 'comment_img_description.comment_id', ], 'joins' => [ - 'temp_img_description' => [ 'LEFT JOIN', 'temp_img_description.imgcomment_name = img_name' ], 'comment_img_description' => [ 'LEFT JOIN', - // phpcs:ignore Generic.Files.LineLength - 'comment_img_description.comment_id = (CASE WHEN img_description_id != 0 THEN img_description_id ELSE temp_img_description.imgcomment_description_id END)', + 'comment_img_description.comment_id = img_description_id', ], ], ], @@ -316,7 +310,6 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'Image, write-new' => [ MIGRATION_WRITE_NEW, 'img_description', [ 'tables' => [ - 'temp_img_description' => 'image_comment_temp', 'comment_img_description' => 'comment', ], 'fields' => [ @@ -325,10 +318,8 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'img_description_cid' => 'comment_img_description.comment_id', ], 'joins' => [ - 'temp_img_description' => [ 'LEFT JOIN', 'temp_img_description.imgcomment_name = img_name' ], 'comment_img_description' => [ 'LEFT JOIN', - // phpcs:ignore Generic.Files.LineLength - 'comment_img_description.comment_id = (CASE WHEN img_description_id != 0 THEN img_description_id ELSE temp_img_description.imgcomment_description_id END)', + 'comment_img_description.comment_id = img_description_id', ], ], ], @@ -336,7 +327,6 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'Image, new' => [ MIGRATION_NEW, 'img_description', [ 'tables' => [ - 'temp_img_description' => 'image_comment_temp', 'comment_img_description' => 'comment', ], 'fields' => [ @@ -345,10 +335,8 @@ class CommentStoreTest extends MediaWikiLangTestCase { 'img_description_cid' => 'comment_img_description.comment_id', ], 'joins' => [ - 'temp_img_description' => [ 'LEFT JOIN', 'temp_img_description.imgcomment_name = img_name' ], 'comment_img_description' => [ 'JOIN', - // phpcs:ignore Generic.Files.LineLength - 'comment_img_description.comment_id = (CASE WHEN img_description_id != 0 THEN img_description_id ELSE temp_img_description.imgcomment_description_id END)', + 'comment_img_description.comment_id = img_description_id', ], ], ], -- 2.20.1