File moves throw error when using postgres
Closed, ResolvedPublic
Actions

Description

@0xAFFF notes:
If I try to rename a file I get this SQL error:

ERROR:  FOR UPDATE is not allowed with aggregate functions
STATEMENT:  SELECT /* LocalFileMoveBatch::verifyDBUpdates  */  COUNT(*)  FROM "oldimage"    WHERE oi_name = 'Ssh-schlüssel-zu-gitlab-hinzufügen.webm'  LIMIT 1   FOR UPDATE

The used software versions:

Softare	Version
MediaWiki	1.28.0
PHP	7.0.13 (apache2handler)
PostgreSQL	9.6.1
TimedMediaHandler	0.5.0 (64461e9) 21:18, 25. Okt. 2016
MwEmbedSupport	0.3.0 (2e24ff4) 00:03, 25. Okt. 2016

@TheDJ notes
This part of the code was recently changed by @aaron in ad95e0182356224dce27f53f7bbea24ffbcb22a7, so I think it's likely to be a regression in 1.28

This was originally reported at T157424: TimedMediaHandler does not support PostgreSQL

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	rdbms: make select() warn when FOR UPDATE is used with aggregation	mediawiki/core	master	+75 -7
	Fix LocalFileMoveBatch query that was incompatibile with Postgres	mediawiki/core	master	+2 -2

Customize query in gerrit

Related Objects

Mentioned In: T171390: Exception: Wikimedia\Rdbms\DBUnexpectedError when attempting to move within File: namespace
Mentioned Here: rMWad95e0182356: Avoid use of DB rollback() in LocalFileMoveBatch
rEMES2e24ff47a3bf: Whoops, track not trace
rETMH64461e9ec3d4: Localisation updates from https://translatewiki.net.
T157424: TimedMediaHandler does not support PostgreSQL

Event Timeline

TheDJ created this task.Mar 20 2017, 1:19 PM

Restricted Application added projects: Multimedia, Commons. · View Herald TranscriptMar 20 2017, 1:19 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Paladox subscribed.Mar 20 2017, 1:23 PM

TheDJ updated the task description. (Show Details)Mar 20 2017, 1:23 PM

Paladox added a project: MW-1.28-release.Mar 20 2017, 1:24 PM

Paladox updated the task description. (Show Details)

Paladox added a project: MediaWiki-libs-Rdbms.Mar 20 2017, 6:17 PM

Krinkle moved this task from Untriaged to Usage problem on the MediaWiki-libs-Rdbms board.May 8 2017, 12:58 AM

zhuyifei1999 removed a project: Commons.Jul 25 2017, 9:41 AM

We're doing triaging this morning.

File moving is still broken in 1.29.1:

2017-10-05 16:38:41 SELECT /* LocalFileMoveBatch::verifyDBUpdates  */ COUNT(*)  FROM "oldimage"    WHERE oi_name = 'Awuwei.png'  LIMIT 1   FOR UPDATE
2017-10-05 16:38:41 ROLLBACK /* Wikimedia\Rdbms\DatabasePostgres::reportQueryError  */
2017-10-05 16:38:41 SQL ERROR: ERROR:  FOR UPDATE is not allowed with aggregate functions

COUNT() is indeed an aggregate function and locking COUNT() rows is impossible.

In includes/filerepo/file/LocalFile.php

$oldRowCount = $dbw->selectField(
  'oldimage',
  'COUNT(*)',
  [ 'oi_name' => $this->oldName ],
  __METHOD__,
  [ 'FOR UPDATE' ]
);

The [ 'FOR UPDATE' ] line should be removed as even if it could lock rows, it would not be needed in this case. This is likely a copy and paste mistake. Without it everything works fine.

MaxSem removed a project: MW-1.28-release.Nov 2 2017, 8:19 PM

Ciencia_Al_Poder mentioned this in T171390: Exception: Wikimedia\Rdbms\DBUnexpectedError when attempting to move within File: namespace.Nov 8 2017, 3:05 PM

Krinkle merged a task: T171390: Exception: Wikimedia\Rdbms\DBUnexpectedError when attempting to move within File: namespace.Apr 10 2018, 3:41 PM

Krinkle added subscribers: Ciencia_Al_Poder, Marostegui.

Change 425731 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] rdbms: make selectField() handle COUNT() with FOR UPDATE better

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

gerritbot added a project: Patch-For-Review.Apr 12 2018, 12:44 AM

Change 425760 had a related patch set uploaded (by Aaron Schulz; owner: Aaron Schulz):
[mediawiki/core@master] Fix LocalFileMoveBatch query that was incompatibile with Postgres

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

Change 425760 merged by jenkins-bot:
[mediawiki/core@master] Fix LocalFileMoveBatch query that was incompatibile with Postgres

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-04-17 (1.31.0-wmf.30)).Apr 12 2018, 5:00 PM

Change 425731 merged by jenkins-bot:
[mediawiki/core@master] rdbms: make select() warn when FOR UPDATE is used with aggregation

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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-04-24 (1.32.0-wmf.1)).Apr 23 2018, 9:00 PM

aaron closed this task as Resolved.Apr 23 2018, 10:10 PM

aaron claimed this task.

Krinkle edited projects, added Performance-Team; removed Patch-For-Review.Jun 16 2018, 10:38 PM