MigrateActors: Don't delete log_search rows when migrating
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 7 Feb 2019 15:27:06 +0000 (10:27 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Fri, 15 Feb 2019 19:54:29 +0000 (14:54 -0500)
When I4764c1c78 switched from being run during read-both/write-new to
write-both/read-old, we should have also removed the code that
blanked/deleted the old rows. That was done for the main migration, but
was overlooked for log_search.

Bug: T215464
Change-Id: Icbba54dbd57fe0fa07ea0f6dcdde30089f067ace

RELEASE-NOTES-1.33
maintenance/includes/MigrateActors.php

index 299cd94..59f4615 100644 (file)
@@ -22,6 +22,10 @@ production.
   MediaWiki 1.32, now defaults to MIGRATION_NEW instead of MIGRATION_WRITE_BOTH.
 * Special:ActiveUsers will no longer filter out users who became inactive since
   the last time the active users query cache was updated.
+* If you ran migrateActors.php using an older version of MediaWiki and want to
+  run your wiki with $wgActorTableSchemaMigrationStage SCHEMA_COMPAT_READ_OLD,
+  note that log_search rows needed to find revision deletions by target user
+  were incorrectly deleted. See T215464 for details.
 
 ==== Removed configuration ====
 * (T199334) $wgTagStatisticsNewTable — This temporary setting, added in
index ceba9b5..813f88e 100644 (file)
@@ -413,13 +413,13 @@ class MigrateActors extends LoggedUpdateMaintenance {
        protected function migrateLogSearch() {
                $complainedAboutUsers = [];
 
-               $primaryKey = [ 'ls_field', 'ls_value' ];
+               $primaryKey = [ 'ls_value', 'ls_log_id' ];
                $pkFilter = array_flip( $primaryKey );
                $this->output( "Beginning migration of log_search\n" );
                wfWaitForSlaves();
 
                $dbw = $this->getDB( DB_MASTER );
-               $countUpdated = 0;
+               $countInserted = 0;
                $countActors = 0;
                $countErrors = 0;
 
@@ -427,72 +427,44 @@ class MigrateActors extends LoggedUpdateMaintenance {
                while ( true ) {
                        // Fetch the rows needing update
                        $res = $dbw->select(
+                               [ 'log_search', 'actor' ],
+                               [ 'ls_value', 'ls_log_id', 'actor_id' ],
                                [
-                                       'ls' => $dbw->buildSelectSubquery(
-                                               'log_search',
-                                               'ls_value',
-                                               [
-                                                       'ls_field' => 'target_author_id',
-                                                       $next
-                                               ],
-                                               __METHOD__,
-                                               [
-                                                       'DISTINCT',
-                                                       'ORDER BY' => [ 'ls_value' ],
-                                                       'LIMIT' => $this->mBatchSize,
-                                               ]
-                                       ),
-                                       'actor'
+                                       'ls_field' => 'target_author_id',
+                                       $next
                                ],
+                               __METHOD__,
                                [
-                                       'ls_field' => $dbw->addQuotes( 'target_author_id' ),
-                                       'ls_value',
-                                       'actor_id'
+                                       'ORDER BY' => $primaryKey,
+                                       'LIMIT' => $this->mBatchSize,
                                ],
-                               [],
-                               __METHOD__,
-                               [],
                                [ 'actor' => [ 'LEFT JOIN', 'ls_value = ' . $dbw->buildStringCast( 'actor_user' ) ] ]
                        );
                        if ( !$res->numRows() ) {
                                break;
                        }
 
-                       // Update the rows
-                       $del = [];
+                       // Insert a 'target_author_actor' for each 'target_author_id'
+                       $ins = [];
                        foreach ( $res as $row ) {
                                $lastRow = $row;
                                if ( !$row->actor_id ) {
                                        list( , $display ) = $this->makeNextCond( $dbw, $primaryKey, $row );
-                                       $this->error( "No actor for row with $display\n" );
+                                       $this->error( "No actor for target_author_id row with $display\n" );
                                        $countErrors++;
                                        continue;
                                }
-                               $dbw->update(
-                                       'log_search',
-                                       [
-                                               'ls_field' => 'target_author_actor',
-                                               'ls_value' => $row->actor_id,
-                                       ],
-                                       [
-                                               'ls_field' => $row->ls_field,
-                                               'ls_value' => $row->ls_value,
-                                       ],
-                                       __METHOD__,
-                                       [ 'IGNORE' ]
-                               );
-                               $countUpdated += $dbw->affectedRows();
-                               $del[] = $row->ls_value;
-                       }
-                       if ( $del ) {
-                               $dbw->delete(
-                                       'log_search', [ 'ls_field' => 'target_author_id', 'ls_value' => $del ], __METHOD__
-                               );
-                               $countUpdated += $dbw->affectedRows();
+                               $ins[] = [
+                                       'ls_field' => 'target_author_actor',
+                                       'ls_value' => $row->actor_id,
+                                       'ls_log_id' => $row->ls_log_id,
+                               ];
                        }
+                       $dbw->insert( 'log_search', $ins, __METHOD__, [ 'IGNORE' ] );
+                       $countInserted += $dbw->affectedRows();
 
                        list( $next, $display ) = $this->makeNextCond( $dbw, $primaryKey, $lastRow );
-                       $this->output( "... $display\n" );
+                       $this->output( "... target_author_id, $display\n" );
                        wfWaitForSlaves();
                }
 
@@ -500,31 +472,17 @@ class MigrateActors extends LoggedUpdateMaintenance {
                while ( true ) {
                        // Fetch the rows needing update
                        $res = $dbw->select(
+                               [ 'log_search', 'actor' ],
+                               [ 'ls_value', 'ls_log_id', 'actor_id' ],
                                [
-                                       'ls' => $dbw->buildSelectSubquery(
-                                               'log_search',
-                                               'ls_value',
-                                               [
-                                                       'ls_field' => 'target_author_ip',
-                                                       $next
-                                               ],
-                                               __METHOD__,
-                                               [
-                                                       'DISTINCT',
-                                                       'ORDER BY' => [ 'ls_value' ],
-                                                       'LIMIT' => $this->mBatchSize,
-                                               ]
-                                       ),
-                                       'actor'
+                                       'ls_field' => 'target_author_ip',
+                                       $next
                                ],
+                               __METHOD__,
                                [
-                                       'ls_field' => $dbw->addQuotes( 'target_author_ip' ),
-                                       'ls_value',
-                                       'actor_id'
+                                       'ORDER BY' => $primaryKey,
+                                       'LIMIT' => $this->mBatchSize,
                                ],
-                               [],
-                               __METHOD__,
-                               [],
                                [ 'actor' => [ 'LEFT JOIN', 'ls_value = actor_name' ] ]
                        );
                        if ( !$res->numRows() ) {
@@ -538,45 +496,31 @@ class MigrateActors extends LoggedUpdateMaintenance {
                                $dbw, 'ls_value', $rows, $complainedAboutUsers, $countErrors
                        );
 
-                       // Update the rows
-                       $del = [];
+                       // Insert a 'target_author_actor' for each 'target_author_ip'
+                       $ins = [];
                        foreach ( $rows as $row ) {
                                if ( !$row->actor_id ) {
                                        list( , $display ) = $this->makeNextCond( $dbw, $primaryKey, $row );
-                                       $this->error( "Could not make actor for row with $display\n" );
+                                       $this->error( "Could not make actor for target_author_ip row with $display\n" );
                                        $countErrors++;
                                        continue;
                                }
-                               $dbw->update(
-                                       'log_search',
-                                       [
-                                               'ls_field' => 'target_author_actor',
-                                               'ls_value' => $row->actor_id,
-                                       ],
-                                       [
-                                               'ls_field' => $row->ls_field,
-                                               'ls_value' => $row->ls_value,
-                                       ],
-                                       __METHOD__,
-                                       [ 'IGNORE' ]
-                               );
-                               $countUpdated += $dbw->affectedRows();
-                               $del[] = $row->ls_value;
-                       }
-                       if ( $del ) {
-                               $dbw->delete(
-                                       'log_search', [ 'ls_field' => 'target_author_ip', 'ls_value' => $del ], __METHOD__
-                               );
-                               $countUpdated += $dbw->affectedRows();
+                               $ins[] = [
+                                       'ls_field' => 'target_author_actor',
+                                       'ls_value' => $row->actor_id,
+                                       'ls_log_id' => $row->ls_log_id,
+                               ];
                        }
+                       $dbw->insert( 'log_search', $ins, __METHOD__, [ 'IGNORE' ] );
+                       $countInserted += $dbw->affectedRows();
 
                        list( $next, $display ) = $this->makeNextCond( $dbw, $primaryKey, $lastRow );
-                       $this->output( "... $display\n" );
+                       $this->output( "... target_author_ip, $display\n" );
                        wfWaitForSlaves();
                }
 
                $this->output(
-                       "Completed migration, updated $countUpdated row(s) with $countActors new actor(s), "
+                       "Completed migration, inserted $countInserted row(s) with $countActors new actor(s), "
                        . "$countErrors error(s)\n"
                );
                return $countErrors;