ImageListPager: Kill that annoying GROUP BY
authorBrad Jorsch <bjorsch@wikimedia.org>
Fri, 4 Jan 2019 22:09:25 +0000 (17:09 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Fri, 4 Jan 2019 22:38:08 +0000 (17:38 -0500)
The GROUP BY is broken in later comment migration stages, we just didn't
notice because it's not used when $wgMiserMode is set. Instead of trying
to fix it, yet again, let's just do the count as a subselect.

While this code is being touched, let's also use Database's aliasing
functionality properly instead of passing "foo AS bar" as integer-keyed
values in $fields (with array_search() being used to change them!), and
let's start using Database->addQuotes() for string literals instead of
hard-coding single quotes.

Bug: T212980
Change-Id: Ide7f67893f625fe03127d4a775642cf0a9cca195

includes/specials/pagers/ImageListPager.php

index 3ddbe08..1794362 100644 (file)
@@ -253,28 +253,28 @@ class ImageListPager extends TablePager {
         * @return array Query info
         */
        protected function getQueryInfoReal( $table ) {
+               $dbr = wfGetDB( DB_REPLICA );
                $prefix = $table === 'oldimage' ? 'oi' : 'img';
 
                $tables = [ $table ];
-               $fields = $this->getFieldNames();
+               $fields = array_keys( $this->getFieldNames() );
+               $fields = array_combine( $fields, $fields );
                unset( $fields['img_description'] );
                unset( $fields['img_user_text'] );
-               $fields = array_keys( $fields );
 
                if ( $table === 'oldimage' ) {
-                       foreach ( $fields as $id => &$field ) {
-                               if ( substr( $field, 0, 4 ) !== 'img_' ) {
-                                       continue;
+                       foreach ( $fields as $id => $field ) {
+                               if ( substr( $id, 0, 4 ) === 'img_' ) {
+                                       $fields[$id] = $prefix . substr( $field, 3 );
                                }
-                               $field = $prefix . substr( $field, 3 ) . ' AS ' . $field;
                        }
-                       $fields[array_search( 'top', $fields )] = "'no' AS top";
+                       $fields['top'] = $dbr->addQuotes( 'no' );
                } else {
                        if ( $this->mShowAll ) {
-                               $fields[array_search( 'top', $fields )] = "'yes' AS top";
+                               $fields['top'] = $dbr->addQuotes( 'yes' );
                        }
                }
-               $fields[array_search( 'thumb', $fields )] = $prefix . '_name AS thumb';
+               $fields['thumb'] = $prefix . '_name';
 
                $options = $join_conds = [];
 
@@ -283,7 +283,7 @@ class ImageListPager extends TablePager {
                $tables += $commentQuery['tables'];
                $fields += $commentQuery['fields'];
                $join_conds += $commentQuery['joins'];
-               $fields['description_field'] = "'{$prefix}_description'";
+               $fields['description_field'] = $dbr->addQuotes( "{$prefix}_description" );
 
                # User fields
                $actorQuery = ActorMigration::newMigration()->getJoin( $prefix . '_user' );
@@ -295,20 +295,13 @@ class ImageListPager extends TablePager {
 
                # Depends on $wgMiserMode
                # Will also not happen if mShowAll is true.
-               if ( isset( $this->mFieldNames['count'] ) ) {
-                       $tables[] = 'oldimage';
-
-                       # Need to rewrite this one
-                       foreach ( $fields as &$field ) {
-                               if ( $field == 'count' ) {
-                                       $field = 'COUNT(oi_archive_name) AS count';
-                               }
-                       }
-                       unset( $field );
-
-                       $columnlist = preg_grep( '/^img/', array_keys( $this->getFieldNames() ) );
-                       $options = [ 'GROUP BY' => array_merge( [ $fields['img_user'] ], $columnlist ) ];
-                       $join_conds['oldimage'] = [ 'LEFT JOIN', 'oi_name = img_name' ];
+               if ( isset( $fields['count'] ) ) {
+                       $fields['count'] = $dbr->buildSelectSubquery(
+                               'oldimage',
+                               'COUNT(oi_archive_name)',
+                               'oi_name = img_name',
+                               __METHOD__
+                       );
                }
 
                return [