Page MenuHomePhabricator
Create Task
Maniphest T217487

Various tests failing with Echo/UserMerge issues
Closed, ResolvedPublic
Actions

Description

18:45:47 1) MergeUserTest::testBasicMerge
18:45:47 === Logs generated by test case
18:45:47 [PHPUnitCommand] [info] End test LessFileCompilationTest::testLessFileCompilation {"private":false}
18:45:47 ===
18:45:47 Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? 
18:45:47 Query: SELECT  echo_event.*  FROM `unittest_echo_notification`,`unittest_echo_event`    WHERE notification_user = '8' AND (notification_event = event_id) AND event_type = 'thank-you-edit'  ORDER BY notification_timestamp ASC 
18:45:47 Function: EchoHooks::onMergeAccountFromTo
18:45:47 Error: 1051 Unknown table 'wikidb.echo_event' (/workspace/db/quibble-mysql-7h2jwcnv/socket)
18:45:47 
18:45:47 
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1515
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1485
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1245
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1712
18:45:47 /workspace/src/extensions/Echo/includes/EchoHooks.php:1416
18:45:47 /workspace/src/includes/deferred/MWCallableUpdate.php:34
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:270
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:216
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:140
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:309
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:104
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:121
18:45:47 /workspace/src/extensions/Echo/includes/EchoHooks.php:1447
18:45:47 /workspace/src/includes/Hooks.php:174
18:45:47 /workspace/src/includes/Hooks.php:202
18:45:47 /workspace/src/extensions/UserMerge/includes/MergeUser.php:424
18:45:47 /workspace/src/extensions/UserMerge/includes/MergeUser.php:50
18:45:47 /workspace/src/extensions/UserMerge/tests/phpunit/MergeUserTest.php:44
18:45:47 /workspace/src/tests/phpunit/MediaWikiTestCase.php:425
18:45:47 /workspace/src/tests/phpunit/phpunit.php:90
18:45:47 /workspace/src/maintenance/doMaintenance.php:94
18:45:47 /workspace/src/tests/phpunit/phpunit.php:129
18:45:47 
18:45:47 2) MergeUserTest::testMergeOfUserGroups
18:45:47 === Logs generated by test case
18:45:47 [PHPUnitCommand] [info] End test LessFileCompilationTest::testLessFileCompilation {"private":false}
18:45:47 ===
18:45:47 Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? 
18:45:47 Query: SELECT  echo_event.*  FROM `unittest_echo_notification`,`unittest_echo_event`    WHERE notification_user = '11' AND (notification_event = event_id) AND event_type = 'thank-you-edit'  ORDER BY notification_timestamp ASC 
18:45:47 Function: EchoHooks::onMergeAccountFromTo
18:45:47 Error: 1051 Unknown table 'wikidb.echo_event' (/workspace/db/quibble-mysql-7h2jwcnv/socket)
18:45:47 
18:45:47 
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1515
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1485
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1245
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1712
18:45:47 /workspace/src/extensions/Echo/includes/EchoHooks.php:1416
18:45:47 /workspace/src/includes/deferred/MWCallableUpdate.php:34
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:270
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:216
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:140
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:309
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:104
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:121
18:45:47 /workspace/src/extensions/Echo/includes/EchoHooks.php:1447
18:45:47 /workspace/src/includes/Hooks.php:174
18:45:47 /workspace/src/includes/Hooks.php:202
18:45:47 /workspace/src/extensions/UserMerge/includes/MergeUser.php:424
18:45:47 /workspace/src/extensions/UserMerge/includes/MergeUser.php:50
18:45:47 /workspace/src/extensions/UserMerge/tests/phpunit/MergeUserTest.php:63
18:45:47 /workspace/src/tests/phpunit/MediaWikiTestCase.php:425
18:45:47 /workspace/src/tests/phpunit/phpunit.php:90
18:45:47 /workspace/src/maintenance/doMaintenance.php:94
18:45:47 /workspace/src/tests/phpunit/phpunit.php:129
18:45:47 
18:45:47 3) MergeUserTest::testMergeEditcount
18:45:47 === Logs generated by test case
18:45:47 [PHPUnitCommand] [info] End test LessFileCompilationTest::testLessFileCompilation {"private":false}
18:45:47 ===
18:45:47 Wikimedia\Rdbms\DBQueryError: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? 
18:45:47 Query: SELECT  echo_event.*  FROM `unittest_echo_notification`,`unittest_echo_event`    WHERE notification_user = '15' AND (notification_event = event_id) AND event_type = 'thank-you-edit'  ORDER BY notification_timestamp ASC 
18:45:47 Function: EchoHooks::onMergeAccountFromTo
18:45:47 Error: 1051 Unknown table 'wikidb.echo_event' (/workspace/db/quibble-mysql-7h2jwcnv/socket)
18:45:47 
18:45:47 
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1515
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1485
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1245
18:45:47 /workspace/src/includes/libs/rdbms/database/Database.php:1712
18:45:47 /workspace/src/extensions/Echo/includes/EchoHooks.php:1416
18:45:47 /workspace/src/includes/deferred/MWCallableUpdate.php:34
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:270
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:216
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:140
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:309
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:104
18:45:47 /workspace/src/includes/deferred/DeferredUpdates.php:121
18:45:47 /workspace/src/extensions/Echo/includes/EchoHooks.php:1447
18:45:47 /workspace/src/includes/Hooks.php:174
18:45:47 /workspace/src/includes/Hooks.php:202
18:45:47 /workspace/src/extensions/UserMerge/includes/MergeUser.php:424
18:45:47 /workspace/src/extensions/UserMerge/includes/MergeUser.php:50
18:45:47 /workspace/src/extensions/UserMerge/tests/phpunit/MergeUserTest.php:104
18:45:47 /workspace/src/tests/phpunit/MediaWikiTestCase.php:425
18:45:47 /workspace/src/tests/phpunit/phpunit.php:90
18:45:47 /workspace/src/maintenance/doMaintenance.php:94
18:45:47 /workspace/src/tests/phpunit/phpunit.php:129

Details

SubjectRepoBranchLines +/-
Fix UserMerge integration for use with table prefixmediawiki/extensions/Echowmf/1.33.0-wmf.19+22 -1
Fix UserMerge integration for use with table prefixmediawiki/extensions/Echomaster+22 -1
Customize query in gerrit

Related Objects

Event Timeline

Reedy created this task.Mar 2 2019, 6:47 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptMar 2 2019, 6:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Reedy triaged this task as High priority.Mar 2 2019, 7:51 PM
Reedy added a project: Release-Engineering-Team.
Umherirrender subscribed.Mar 2 2019, 8:24 PM

https://gerrit.wikimedia.org/r/#/c/integration/config/+/493808/4/zuul/parameter_functions.py added UserMerge as dependency to CentralAuth, which is a dependency of Echo

gerritbot subscribed.Mar 2 2019, 8:25 PM

Change 493880 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/Echo@master] Use explicit columns and avoid SELECT *

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

gerritbot added a project: Patch-For-Review.Mar 2 2019, 8:25 PM
Reedy lowered the priority of this task from High to Medium.Mar 2 2019, 8:34 PM
Daimona subscribed.Mar 2 2019, 8:59 PM

Thanks for the prompt fix! I'm sorry for the inconvenience, although IMHO it's been useful to discover a potential bug cause.

Umherirrender added a comment.Mar 2 2019, 9:06 PM
In T217487#4995844, @Daimona wrote:

Thanks for the prompt fix! I'm sorry for the inconvenience, although IMHO it's been useful to discover a potential bug cause.

Yes, it was good to add this dependency and find the bug, but it breaks more than only Echo due to other extension using this as depenendcy

Daimona added a comment.Mar 3 2019, 10:54 AM

While I know the CI revert is temporary, I'd like to point out that CheckUser is currently broken because phan has been enabled for it, but now it lacks the needed directories.
I'd also like to seize the opportunity for a remark: it seems very wrong that selecting table_name.* fails because it cannot find the table. I think Database::select() and friends should detect when a table name is used this way and properly add the table prefix to it. Or at the very least, document that this syntax isn't valid, and/or ban it via a CodeSniffer check. One may argue that SELECTs involving * usually aren't the best thing to do, but that doesn't mean that we can leave them broken.
I don't guarantee that I'll get it done myself, but I believe it's very important to address this issue.

Umherirrender added a comment.Mar 3 2019, 1:46 PM

In case of table_name.* the table_name could be the table alias and not the name of the table.
So it is possible to use it this way, but than it needs an explizit alias on the Database::select to avoid the problem on table prefix

I have not tested, but this could work, because table prefix is first added and than checked if table name and alias are equal and optimized:

				$dbw->select(
					[ 'echo_notification', 'echo_event' => 'echo_event' ],
					'echo_event.*',
					[
						'notification_user' => $newUser->getId(),
						'notification_event = event_id',
						'event_type' => 'thank-you-edit',
					],
					$method,
					[ 'ORDER BY' => 'notification_timestamp ASC' ]
				)

With alias

				$dbw->select(
					[ 'echo_notification', 'event' => 'echo_event' ],
					'event.*',
					[
						'notification_user' => $newUser->getId(),
						'notification_event = event_id',
						'event_type' => 'thank-you-edit',
					],
					$method,
					[ 'ORDER BY' => 'notification_timestamp ASC' ]
				)

The best is to avoid SELECT * and only select the fields which are needed.

This seems to complex for phpcs, because it has also to check for return of getQueryInfo or use of variable. The security plugin also has some limitation there.

Daimona added a comment.Mar 3 2019, 2:40 PM

@Umherirrender Your patch for echo is definitely good, and fine for now. I'm trying to see if I can implement the feature in the Database class, although it may be tricky... Yes, avoiding * is the best choice, but it's not always applicable, and as I was saying above we should keep it working anyway.
Agreed on PHPCS, definitely too complicated.

gerritbot added a comment.Mar 3 2019, 3:28 PM

Change 494040 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] [WIP] Add table prefix when using 'table_name.*' in SELECT

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

gerritbot added a comment.Mar 4 2019, 7:55 AM

Change 493880 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Fix UserMerge integration for use with table prefix

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

gerritbot added a comment.Mar 4 2019, 8:28 PM

Change 494304 had a related patch set uploaded (by Reedy; owner: Umherirrender):
[mediawiki/extensions/Echo@wmf/1.33.0-wmf.19] Fix UserMerge integration for use with table prefix

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

gerritbot added a comment.Mar 4 2019, 8:42 PM

Change 494304 merged by jenkins-bot:
[mediawiki/extensions/Echo@wmf/1.33.0-wmf.19] Fix UserMerge integration for use with table prefix

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

Stashbot subscribed.Mar 4 2019, 8:44 PM

Mentioned in SAL (#wikimedia-operations) [2019-03-04T20:44:39Z] <reedy@deploy1001> Synchronized php-1.33.0-wmf.19/extensions/Echo/: T217487 (duration: 00m 53s)

Reedy renamed this task from OAuth tests failing with Echo/UserMerge issues to Various tests failing with Echo/UserMerge issues.Mar 4 2019, 8:45 PM
Reedy removed a project: MediaWiki-extensions-OAuth.
Lucas_Werkmeister_WMDE mentioned this in T217553: Merge blocker: MediaWiki\MassMessage\MassMessageJobTest::testLQTMessageSending.Mar 5 2019, 10:24 AM
ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.21; 2019-03-12).Mar 5 2019, 4:01 PM
JTannerWMF closed this task as Resolved.Mar 5 2019, 7:39 PM
JTannerWMF claimed this task.
Daimona removed a project: Patch-For-Review.Mar 5 2019, 7:40 PM

The patch for changing Database::select was still pending, but it's better to move it to a separate task.

Daimona mentioned this in T217743: Database::selectSQLText should prefix 'table_name.*'.Mar 6 2019, 10:27 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL