Page MenuHomePhabricator

MysqlDatabaseBase::tableExists returns wrong results because method tableName returns the table name prefixed with the database if the table is a shared table
Closed, ResolvedPublic

Description

I ran into this problem trying to update shared tables with update.php, and after digging deep, I finally found out that Database::tableName returns the table name prefixed by the database it resides in when that table is shared. MysqlDatabaseBase::tableExists places that directly in the SQL query. This way tableExists reports that the table does not exist while running a SHOW TABLES; query on the shared database reports the table as existing. Manually inputting the SQL query in the database confirms that the query returns 0 results. The reason update.php fails, is because it relies on the tableExists method to check if a table exists, and with that knowledge can determine if it should create the table, or skip the patching process. So even if update.php ran without throwing an error, it still has not patched the global tables. The only indication to this is a message mentioning that TABLENAME table does not exist, skipping [...] patch.

Environment

  • MediaWiki-Vagrant
  • MySQL
  • MediaWiki 1.30 - master branch

Example

Table name: user_properties
Database name: wiki
Call to Database::tableName: $db->tableName( 'user_properties', 'raw' );
Output from Database::tableName: wiki.user_properties
SQL query sent to the database: SHOW TABLES LIKE 'wiki.user\_properties';

Reproducing (on MediaWiki-Vagrant)

Set $wgSharedDB to wiki (MediaWiki-Vagrant's default wiki database) and try to run mwscript update.php --doshared

Solution

Staring at https://dev.mysql.com/doc/refman/5.7/en/show-tables.html I'm assuming the query should instead be SHOW TABLES FROM wiki LIKE 'user\_properties';

Event Timeline

Change 359961 had a related patch set uploaded (by Mainframe98; owner: Mainframe98):
[mediawiki/core@master] Fix the tableExists method of MysqlBase

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

Change 359961 merged by jenkins-bot:
[mediawiki/core@master] Fix the tableExists method of MysqlBase

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

Krinkle closed this task as Resolved.EditedJun 23 2017, 1:38 AM
Krinkle assigned this task to Mainframe98.
Krinkle triaged this task as High priority.
Krinkle removed a project: Patch-For-Review.
Krinkle moved this task from Untriaged to Rdbms library on the MediaWiki-libs-Rdbms board.
Krinkle subscribed.

Thanks @Mainframe98 !

This change just broke all of our integration tests with:

[fe030290b50a4b682633a6a0] [no req]   Wikimedia\Rdbms\DBQueryError from line 1145 of ...\includes\libs\rdbms\database\Database.php: A database query error has occurred. Did you forget to run your application's database schema updater after upgrading?
Query: CREATE  TABLE `unittest_archive` (LIKE `archive`)
Function: Wikimedia\Rdbms\DatabaseMysqlBase::duplicateTableStructure
Error: 1050 Table 'unittest_archive' already exists (localhost)

Backtrace:
#0 ...\includes\libs\rdbms\database\Database.php(975): Wikimedia\Rdbms\Database->reportQueryError(string, integer, string, string, boolean)
#1 ...\includes\libs\rdbms\database\DatabaseMysqlBase.php(1245): Wikimedia\Rdbms\Database->query(string, string)
#2 ...\includes\db\CloneDatabase.php(112): Wikimedia\Rdbms\DatabaseMysqlBase->duplicateTableStructure(string, string, boolean)
MediaWiki	1.30.0-alpha (f0f0365)
PHP	7.1.1 (apache2handler)
MariaDB	10.1.21-MariaDB
ICU	57.1

Reverting to any merge before this PR resolves this issue hence it is very likely that this PR introduce some kind of regression.

Doing a quick var_dump on $encLike = $this->escapeLikeInternal( $table, '\\' ); reveals that before the PR it would return "unittest\_archive" and now after the change it contains "unittest\_unittest\_archive".

This change just broke all of our integration tests

Which tests specifically fail for you?

Which tests specifically fail for you?

All of them but it doesn't really matter now since this was fixed and addressed with https://github.com/wikimedia/mediawiki/commit/0352fe44f10fa2a20027b6ca7d583232019781cb.

That fix broke CentralAuth tests.