Drop the image_comment_temp table
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 7 Mar 2018 20:25:53 +0000 (15:25 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Wed, 14 Nov 2018 20:04:31 +0000 (15:04 -0500)
It is no longer used.

Bug: T188132
Change-Id: Ic8efeddc030f48e82ba861926121b64eca37d169

RELEASE-NOTES-1.33
includes/CommentStore.php
includes/filerepo/file/LocalFile.php
includes/installer/DatabaseUpdater.php
maintenance/mssql/tables.sql
maintenance/oracle/tables.sql
maintenance/postgres/tables.sql
maintenance/tables.sql
tests/parser/ParserTestRunner.php
tests/phpunit/includes/CommentStoreTest.php

index 7bbb165..c573a59 100644 (file)
@@ -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 ==
index 7a2726f..cba7a15 100644 (file)
@@ -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";
index ebbc8f8..95ee4f4 100644 (file)
@@ -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(
index 925fc5a..43000b8 100644 (file)
@@ -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' );
+                       }
                }
        }
 
index 63e0aa8..49115de 100644 (file)
@@ -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.
index 6e76851..1ccaabf 100644 (file)
@@ -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,
index 003204d..96a0617 100644 (file)
@@ -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,
index c46e4c6..3c8b598 100644 (file)
@@ -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.
index 3dee521..bc5f1fc 100644 (file)
@@ -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 ) {
index bc1f101..4360343 100644 (file)
@@ -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',
                                                ],
                                        ],
                                ],