Make archive.ar_rev_id unique

To follow up I39b0825c, this change replaces the existing non-unique
index on the column with a unique index, to help avoid some of these
sort of bugs in the future.

Bug: T193180
Change-Id: I932478c9c6a13210bc9dff75286d0f08da56682c

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index d0783b6..11e306a 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -24,6 +24,7 @@ production.
 * New configuration variable has been added: $wgCookieSetOnIpBlock.
   This determines whether to set a cookie when an IP user is blocked. Doing so means
   that a blocked user, even after moving to a new IP address, will still be blocked.
+* The archive table's ar_rev_id field is now unique.
 
 === New features in 1.32 ===
 * (T112474) Generalized the ResourceLoader mechanism for overriding modules

diff --git a/includes/installer/MssqlUpdater.php b/includes/installer/MssqlUpdater.php
index 44b4e30..cfa21f4 100644 (file)
--- a/includes/installer/MssqlUpdater.php
+++ b/includes/installer/MssqlUpdater.php
@@ -136,6 +136,7 @@ class MssqlUpdater extends DatabaseUpdater {
                                'patch-externallinks-el_index_60-drop-default.sql' ],
                        [ 'runMaintenance', DeduplicateArchiveRevId::class, 'maintenance/deduplicateArchiveRevId.php' ],
                        [ 'addField', 'change_tag', 'ct_tag_id', 'patch-change_tag-tag_id.sql' ],
+                       [ 'addIndex', 'archive', 'ar_revid_uniq', 'patch-archive-ar_rev_id-unique.sql' ],
                ];
        }
 

diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php
index d70edea..476e729 100644 (file)
--- a/includes/installer/MysqlUpdater.php
+++ b/includes/installer/MysqlUpdater.php
@@ -356,6 +356,7 @@ class MysqlUpdater extends DatabaseUpdater {
                                'patch-externallinks-el_index_60-drop-default.sql' ],
                        [ 'runMaintenance', DeduplicateArchiveRevId::class, 'maintenance/deduplicateArchiveRevId.php' ],
                        [ 'addField', 'change_tag', 'ct_tag_id', 'patch-change_tag-tag_id.sql' ],
+                       [ 'addIndex', 'archive', 'ar_revid_uniq', 'patch-archive-ar_rev_id-unique.sql' ],
                ];
        }
 

diff --git a/includes/installer/OracleUpdater.php b/includes/installer/OracleUpdater.php
index 965187a..9d2fdc6 100644 (file)
--- a/includes/installer/OracleUpdater.php
+++ b/includes/installer/OracleUpdater.php
@@ -153,6 +153,7 @@ class OracleUpdater extends DatabaseUpdater {
                        [ 'populateExternallinksIndex60' ],
                        [ 'runMaintenance', DeduplicateArchiveRevId::class, 'maintenance/deduplicateArchiveRevId.php' ],
                        [ 'addField', 'change_tag', 'ct_tag_id', 'patch-change_tag-tag_id.sql' ],
+                       [ 'addIndex', 'archive', 'ar_revid_uniq', 'patch-archive-ar_rev_id-unique.sql' ],
 
                        // KEEP THIS AT THE BOTTOM!!
                        [ 'doRebuildDuplicateFunction' ],

diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php
index 49419ea..dc1ffdb 100644 (file)
--- a/includes/installer/PostgresUpdater.php
+++ b/includes/installer/PostgresUpdater.php
@@ -582,6 +582,8 @@ class PostgresUpdater extends DatabaseUpdater {
                                'change_tag_tag_id_id',
                                '( ct_tag_id, ct_rc_id, ct_rev_id, ct_log_id )'
                        ],
+                       [ 'addPgIndex', 'archive', 'ar_revid_uniq', '(ar_rev_id)', 'unique' ],
+                       [ 'dropPgIndex', 'archive', 'ar_revid' ], // Probably doesn't exist, but do it anyway.
                ];
        }
 
@@ -959,12 +961,13 @@ END;
                }
        }
 
-       public function addPgIndex( $table, $index, $type ) {
+       public function addPgIndex( $table, $index, $type, $unique = false ) {
                if ( $this->db->indexExists( $table, $index ) ) {
                        $this->output( "...index '$index' on table '$table' already exists\n" );
                } else {
                        $this->output( "Creating index '$index' on table '$table' $type\n" );
-                       $this->db->query( "CREATE INDEX $index ON $table $type" );
+                       $unique = $unique ? 'UNIQUE' : '';
+                       $this->db->query( "CREATE $unique INDEX $index ON $table $type" );
                }
        }
 

diff --git a/includes/installer/SqliteUpdater.php b/includes/installer/SqliteUpdater.php
index 2b27d37..2a67a0a 100644 (file)
--- a/includes/installer/SqliteUpdater.php
+++ b/includes/installer/SqliteUpdater.php
@@ -220,6 +220,7 @@ class SqliteUpdater extends DatabaseUpdater {
                                'patch-externallinks-el_index_60-drop-default.sql' ],
                        [ 'runMaintenance', DeduplicateArchiveRevId::class, 'maintenance/deduplicateArchiveRevId.php' ],
                        [ 'addField', 'change_tag', 'ct_tag_id', 'patch-change_tag-tag_id.sql' ],
+                       [ 'addIndex', 'archive', 'ar_revid_uniq', 'patch-archive-ar_rev_id-unique.sql' ],
                ];
        }
 

diff --git a/maintenance/archives/patch-archive-ar_rev_id-unique.sql b/maintenance/archives/patch-archive-ar_rev_id-unique.sql
new file mode 100644 (file)
index 0000000..0f07627
--- /dev/null
+++ b/maintenance/archives/patch-archive-ar_rev_id-unique.sql
@@ -0,0 +1,4 @@
+-- T193180: ar_rev_id should be unique
+
+CREATE UNIQUE INDEX /*i*/ar_revid_uniq ON /*_*/archive (ar_rev_id);
+DROP INDEX /*i*/ar_revid ON /*_*/archive;

diff --git a/maintenance/mssql/tables.sql b/maintenance/mssql/tables.sql
index fbe207d..f7db574 100644 (file)
--- a/maintenance/mssql/tables.sql
+++ b/maintenance/mssql/tables.sql
@@ -290,7 +290,7 @@ CREATE TABLE /*_*/archive (
 CREATE INDEX /*i*/name_title_timestamp ON /*_*/archive (ar_namespace,ar_title,ar_timestamp);
 CREATE INDEX /*i*/ar_usertext_timestamp ON /*_*/archive (ar_user_text,ar_timestamp);
 CREATE INDEX /*i*/ar_actor_timestamp ON /*_*/archive (ar_actor,ar_timestamp);
-CREATE INDEX /*i*/ar_revid ON /*_*/archive (ar_rev_id);
+CREATE UNIQUE INDEX /*i*/ar_revid_uniq ON /*_*/archive (ar_rev_id);
 
 
 --

diff --git a/maintenance/oracle/archives/patch-archive-ar_rev_id-unique.sql b/maintenance/oracle/archives/patch-archive-ar_rev_id-unique.sql
new file mode 100644 (file)
index 0000000..c1cccc2
--- /dev/null
+++ b/maintenance/oracle/archives/patch-archive-ar_rev_id-unique.sql
@@ -0,0 +1,6 @@
+-- T193180: ar_rev_id should be unique
+
+define mw_prefix='{$wgDBprefix}';
+
+CREATE UNIQUE INDEX &mw_prefix.archive_i04 ON &mw_prefix.archive (ar_rev_id);
+DROP INDEX &mw_prefix.archive_i03;

diff --git a/maintenance/oracle/tables.sql b/maintenance/oracle/tables.sql
index 6e36752..612f089 100644 (file)
--- a/maintenance/oracle/tables.sql
+++ b/maintenance/oracle/tables.sql
@@ -271,7 +271,7 @@ ALTER TABLE &mw_prefix.archive ADD CONSTRAINT &mw_prefix.archive_fk2 FOREIGN KEY
 CREATE INDEX &mw_prefix.archive_i01 ON &mw_prefix.archive (ar_namespace,ar_title,ar_timestamp);
 CREATE INDEX &mw_prefix.archive_i02 ON &mw_prefix.archive (ar_user_text,ar_timestamp);
 CREATE INDEX &mw_prefix.ar_actor_timestamp ON &mw_prefix.archive (ar_actor,ar_timestamp);
-CREATE INDEX &mw_prefix.archive_i03 ON &mw_prefix.archive (ar_rev_id);
+CREATE UNIQUE INDEX &mw_prefix.archive_i04 ON &mw_prefix.archive (ar_rev_id);
 /*$mw$*/
 CREATE TRIGGER &mw_prefix.archive_seq_trg BEFORE INSERT ON &mw_prefix.archive
        FOR EACH ROW WHEN (new.ar_id IS NULL)

diff --git a/maintenance/postgres/tables.sql b/maintenance/postgres/tables.sql
index cf0eb2f..2f56772 100644 (file)
--- a/maintenance/postgres/tables.sql
+++ b/maintenance/postgres/tables.sql
@@ -263,6 +263,7 @@ ALTER SEQUENCE archive_ar_id_seq OWNED BY archive.ar_id;
 CREATE INDEX archive_name_title_timestamp ON archive (ar_namespace,ar_title,ar_timestamp);
 CREATE INDEX archive_user_text            ON archive (ar_user_text);
 CREATE INDEX archive_actor                ON archive (ar_actor);
+CREATE UNIQUE INDEX ar_revid_uniq ON archive (ar_rev_id);
 
 
 CREATE TABLE slots (

diff --git a/maintenance/sqlite/archives/patch-archive-ar_rev_id-unique.sql b/maintenance/sqlite/archives/patch-archive-ar_rev_id-unique.sql
new file mode 100644 (file)
index 0000000..9677dbb
--- /dev/null
+++ b/maintenance/sqlite/archives/patch-archive-ar_rev_id-unique.sql
@@ -0,0 +1,4 @@
+-- T193180: ar_rev_id should be unique
+
+CREATE UNIQUE INDEX /*i*/ar_revid_uniq ON /*_*/archive (ar_rev_id);
+DROP INDEX /*i*/ar_revid;

diff --git a/maintenance/tables.sql b/maintenance/tables.sql
index 53c1529..d8a47cb 100644 (file)
--- a/maintenance/tables.sql
+++ b/maintenance/tables.sql
@@ -667,7 +667,7 @@ CREATE INDEX /*i*/ar_actor_timestamp ON /*_*/archive (ar_actor,ar_timestamp);
 
 -- Index for linking archive rows with tables that normally link with revision
 -- rows, such as change_tag.
-CREATE INDEX /*i*/ar_revid ON /*_*/archive (ar_rev_id);
+CREATE UNIQUE INDEX /*i*/ar_revid_uniq ON /*_*/archive (ar_rev_id);
 
 --
 -- Slots represent an n:m relation between revisions and content objects.

diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php
index da5cfb1..b4707e6 100644 (file)
--- a/tests/phpunit/MediaWikiTestCase.php
+++ b/tests/phpunit/MediaWikiTestCase.php
@@ -1543,7 +1543,7 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                if ( $db ) {
                        $userTables = [ 'user', 'user_groups', 'user_properties', 'actor' ];
                        $pageTables = [ 'page', 'revision', 'ip_changes', 'revision_comment_temp',
-                               'revision_actor_temp', 'comment' ];
+                               'revision_actor_temp', 'comment', 'archive' ];
                        $coreDBDataTables = array_merge( $userTables, $pageTables );
 
                        // If any of the user or page tables were marked as used, we should clear all of them.