hidemyself/hidebyothers: Use rc_user_text since there is an index
authorMatthew Flaschen <mflaschen@wikimedia.org>
Mon, 27 Mar 2017 21:06:01 +0000 (17:06 -0400)
committerRoan Kattouw <roan.kattouw@gmail.com>
Tue, 28 Mar 2017 19:40:56 +0000 (15:40 -0400)
hidebyothers was extremely slow (on large data sets) due to the
lack of an index on rc_user.  To fix this, changed to use rc_user_text.

hidemyself seems to be fine (assuming normal usage patterns), but
to avoid edge cases and ensure full coverage, it's been changed as
well.

I'll inquire about adding an index for this.

Bug: T161557
Change-Id: I61efe11de12e8ab6c01e8d913cdeda471132a6ee

includes/specialpage/ChangesListSpecialPage.php
tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php

index 8e9629d..1832233 100644 (file)
@@ -177,11 +177,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                                                        &$query_options, &$join_conds ) {
 
                                                        $user = $ctx->getUser();
-                                                       if ( $user->getId() ) {
-                                                               $conds[] = 'rc_user != ' . $dbr->addQuotes( $user->getId() );
-                                                       } else {
-                                                               $conds[] = 'rc_user_text != ' . $dbr->addQuotes( $user->getName() );
-                                                       }
+                                                       $conds[] = 'rc_user_text != ' . $dbr->addQuotes( $user->getName() );
                                                },
                                                'cssClassSuffix' => 'self',
                                                'isRowApplicableCallable' => function ( $ctx, $rc ) {
@@ -197,11 +193,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                                                        &$query_options, &$join_conds ) {
 
                                                        $user = $ctx->getUser();
-                                                       if ( $user->getId() ) {
-                                                               $conds[] = 'rc_user = ' . $dbr->addQuotes( $user->getId() );
-                                                       } else {
-                                                               $conds[] = 'rc_user_text = ' . $dbr->addQuotes( $user->getName() );
-                                                       }
+                                                       $conds[] = 'rc_user_text = ' . $dbr->addQuotes( $user->getName() );
                                                },
                                                'cssClassSuffix' => 'others',
                                                'isRowApplicableCallable' => function ( $ctx, $rc ) {
index c292e97..e10a97f 100644 (file)
@@ -173,7 +173,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                $user = $this->getTestUser()->getUser();
                $this->assertConditions(
                        [ # expected
-                               "rc_user != '{$user->getId()}'",
+                               "rc_user_text != '{$user->getName()}'",
                        ],
                        [
                                'hidemyself' => 1,
@@ -199,7 +199,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                $user = $this->getTestUser()->getUser();
                $this->assertConditions(
                        [ # expected
-                               "rc_user = '{$user->getId()}'",
+                               "rc_user_text = '{$user->getName()}'",
                        ],
                        [
                                'hidebyothers' => 1,
@@ -225,8 +225,8 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                $user = $this->getTestUser()->getUser();
                $this->assertConditions(
                        [ # expected
-                               "rc_user != '{$user->getId()}'",
-                               "rc_user = '{$user->getId()}'",
+                               "rc_user_text != '{$user->getName()}'",
+                               "rc_user_text = '{$user->getName()}'",
                        ],
                        [
                                'hidemyself' => 1,