From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Fri, 4 Jan 2019 22:09:25 +0000 (-0500)
Subject: ImageListPager: Kill that annoying GROUP BY
X-Git-Tag: 1.34.0-rc.0~3091^2
X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=cd3ee70711a8d7fb69f02989f22b403f86aafb96;p=lhc%2Fweb%2Fwiklou.git

ImageListPager: Kill that annoying GROUP BY

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
---

diff --git a/includes/specials/pagers/ImageListPager.php b/includes/specials/pagers/ImageListPager.php
index 3ddbe08d52..1794362461 100644
--- a/includes/specials/pagers/ImageListPager.php
+++ b/includes/specials/pagers/ImageListPager.php
@@ -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 [