Change 443618 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/FlaggedRevs@master] Prevent SQL query error in FlaggedRevsUIHooks::addToFileHistQuery

https://gerrit.wikimedia.org/r/443618

Dear Reedy, dear gerritbot,

sorry, no success. The array $groupBy builded by OldLocalFile::selectFields() contains an empty key. Therefore the double comma still occurs on that query. After unsetting oi_name and oi_timestamp $groupBy shows as follows:
Array (
[1] => oi_archive_name
[2] => oi_size
[3] => oi_width
[4] => oi_height
[5] => oi_metadata
[6] => oi_bits
[7] => oi_media_type
[8] => oi_major_mime
[9] => oi_minor_mime
[10] => oi_user
[11] => oi_user_text
[oi_actor] =>
[13] => oi_deleted
[14] => oi_sha1
[oi_description_text] => oi_description
[oi_description_data] => NULL
[oi_description_cid] => NULL
)

As you can see the key oi_actor has no value and results in double comma on imploding.

How can it have no value? Surely it'd be null or an empty string?

@Anomie looks like it's probably another bug relating to array() + array()

	public static function getQueryInfo( array $options = [] ) {
		$commentQuery = MediaWikiServices::getInstance()->getCommentStore()->getJoin( 'oi_description' );
		$actorQuery = ActorMigration::newMigration()->getJoin( 'oi_user' );
		$ret = [
			'tables' => [ 'oldimage' ] + $commentQuery['tables'] + $actorQuery['tables'],
			'fields' => [
				'oi_name',
				'oi_archive_name',
				'oi_size',
				'oi_width',
				'oi_height',
				'oi_bits',
				'oi_media_type',
				'oi_major_mime',
				'oi_minor_mime',
				'oi_timestamp',
				'oi_deleted',
				'oi_sha1',
			] + $commentQuery['fields'] + $actorQuery['fields'],
			'joins' => $commentQuery['joins'] + $actorQuery['joins'],
		];

All I know, is:
on imploding $groupBy as it is described above, I get oi_name,oi_timestamp,oi_archive_name,oi_size,oi_width,oi_height,oi_metadata,oi_bits,oi_media_type,oi_major_mime,oi_minor_mime,oi_user,oi_user_text,,oi_deleted,oi_sha1,oi_description,NULL,NULL

Maybe on building $groupBy by OldLocalFile::selectFields() an error happens:
return ['oi_user', 'oi_user_text', 'oi_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'oi_actor' : null, 'oi_deleted', 'oi_sha1']
is not returning
[10] => oi_user
[11] => oi_user_text
[oi_actor] => NULL
[13] => oi_deleted
[14] => oi_sha1
as it should.

In FlaggedRevsUI.hooks.php
I changed
$opts['GROUP BY'] = 'oi_name,oi_timestamp,' . implode( ',', $groupBy );
to
$opts['GROUP BY'] = 'oi_name,oi_timestamp,' . implode( ',', array_filter($groupBy) );
Now the odd comma has been driven away.

In T198687#4394348, @Reedy wrote:

@Anomie looks like it's probably another bug relating to array() + array()

It doesn't look like it to me.

In T198687#4394448, @Topsnet-holgerd wrote:

Maybe on building $groupBy by OldLocalFile::selectFields() an error happens:

You should update your FlaggedRevs to the 1.31 version. If it's calling OldLocalFile::selectFields(), you have an older version.

return ['oi_user', 'oi_user_text', 'oi_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'oi_actor' : null, 'oi_deleted', 'oi_sha1']

The null in there should be quoted. I'll submit a patch for that.

Change 443647 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Fix error in various deprecated selectFields() methods

https://gerrit.wikimedia.org/r/443647

Dear Anomie,

thanks a lot for your answer.

You should update your FlaggedRevs to the 1.31 version.

I thought I did this, but will check again tomorrow.

Best regards, Holger

Von: Anomie <no-reply@phabricator.wikimedia.org>
Gesendet: Dienstag, 3. Juli 2018 17:59
An: Holger Drosdek (tops.net) <hd@tops.net>
Betreff: [Maniphest] [Commented On] T198687: FlaggedRevs causes query error in LocalFile::getHistory

Anomie added a comment.

In T198687#4394348https://phabricator.wikimedia.org/T198687#4394348, @Reedyhttps://phabricator.wikimedia.org/p/Reedy/ wrote:

@Anomiehttps://phabricator.wikimedia.org/p/Anomie/ looks like it's probably another bug relating to array() + array()

It doesn't look like it to me.
In T198687#4394448https://phabricator.wikimedia.org/T198687#4394448, @Topsnet-holgerdhttps://phabricator.wikimedia.org/p/Topsnet-holgerd/ wrote:

Maybe on building $groupBy by OldLocalFile::selectFields() an error happens:

You should update your FlaggedRevs to the 1.31 version. If it's calling OldLocalFile::selectFields(), you have an older version.

return ['oi_user', 'oi_user_text', 'oi_actor' => $wgActorTableSchemaMigrationStage > MIGRATION_OLD ? 'oi_actor' : null, 'oi_deleted', 'oi_sha1']

The null in there should be quoted. I'll submit a patch for that.

TASK DETAIL
https://phabricator.wikimedia.org/T198687

EMAIL PREFERENCES
https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Anomie
Cc: Anomie, Reedy, gerritbot, Topsnet-holgerd, Aklapper, Gaboe420, Versusxo, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Baloch007, Darkminds3113, Bsandipan, Lordiis, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, V4switch, Lewizho99, Maathavan, TerraCodes, Poyekhali, Wong128hk, Zache, matthiasmullie, El_Grafo, Dinoguy1000, jeblad, Fabrice_Florin, Jackmcbarn, Steinsplitter, Matanya, Tgr

Change 443662 had a related patch set uploaded (by Reedy; owner: Anomie):
[mediawiki/core@REL1_31] Fix error in various deprecated selectFields() methods

https://gerrit.wikimedia.org/r/443662

Change 443647 merged by jenkins-bot:
[mediawiki/core@master] Fix error in various deprecated selectFields() methods

https://gerrit.wikimedia.org/r/443647

Change 443662 merged by jenkins-bot:
[mediawiki/core@REL1_31] Fix error in various deprecated selectFields() methods

https://gerrit.wikimedia.org/r/443662

Can this be closed? All patches seem to be merged.

Dear Daniel,

yes, this can be closed.

Best regards
Holger Drosdek

Von: daniel <no-reply@phabricator.wikimedia.org>
Gesendet: Mittwoch, 23. Januar 2019 10:04
An: Holger Drosdek (tops.net) <hd@tops.net>
Betreff: [Maniphest] [Commented On] T198687: FlaggedRevs causes query error in LocalFile::getHistory

daniel added a comment.

Can this be closed? All patches seem to be merged.

TASK DETAIL
https://phabricator.wikimedia.org/T198687

EMAIL PREFERENCES
https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: daniel
Cc: daniel, Anomie, Reedy, gerritbot, Topsnet-holgerd, Aklapper, EvanProdromou, CucyNoiD, NebulousIris, Gaboe420, Versusxo, Majesticalreaper22, Giuliamocci, Adrian1985, Cpaulf30, Baloch007, Darkminds3113, Bsandipan, Lordiis, Adik2382, Th3d3v1ls, Ramalepe, Liugev6, V4switch, Lewizho99, Maathavan, TerraCodes, Poyekhali, Agabi10, Cirdan, Wong128hk, Zache, matthiasmullie, El_Grafo, Dinoguy1000, jeblad, Fabrice_Florin, Jackmcbarn, Steinsplitter, Matanya, Tgr