From c84083e413be24988267f846c42dda17c328f4cd Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Fri, 27 Apr 2018 13:11:01 -0400 Subject: [PATCH] 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 --- RELEASE-NOTES-1.32 | 1 + includes/installer/MssqlUpdater.php | 1 + includes/installer/MysqlUpdater.php | 1 + includes/installer/OracleUpdater.php | 1 + includes/installer/PostgresUpdater.php | 7 +++++-- includes/installer/SqliteUpdater.php | 1 + maintenance/archives/patch-archive-ar_rev_id-unique.sql | 4 ++++ maintenance/mssql/tables.sql | 2 +- .../oracle/archives/patch-archive-ar_rev_id-unique.sql | 6 ++++++ maintenance/oracle/tables.sql | 2 +- maintenance/postgres/tables.sql | 1 + .../sqlite/archives/patch-archive-ar_rev_id-unique.sql | 4 ++++ maintenance/tables.sql | 2 +- tests/phpunit/MediaWikiTestCase.php | 2 +- 14 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 maintenance/archives/patch-archive-ar_rev_id-unique.sql create mode 100644 maintenance/oracle/archives/patch-archive-ar_rev_id-unique.sql create mode 100644 maintenance/sqlite/archives/patch-archive-ar_rev_id-unique.sql diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index d0783b6ec3..11e306a32f 100644 --- 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 44b4e30cc2..cfa21f4a82 100644 --- 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 d70edea1d6..476e729363 100644 --- 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 965187a45a..9d2fdc6aeb 100644 --- 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 49419ea978..dc1ffdb645 100644 --- 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 2b27d37bf4..2a67a0a91a 100644 --- 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 index 0000000000..0f07627c0a --- /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 fbe207d174..f7db57408c 100644 --- 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 index 0000000000..c1cccc213e --- /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 6e36752bbe..612f089e51 100644 --- 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 cf0eb2f2ac..2f567720b4 100644 --- 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 index 0000000000..9677dbb3e9 --- /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 53c15290bd..d8a47cb60e 100644 --- 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 da5cfb1527..b4707e657c 100644 --- 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. -- 2.20.1