NewEntitySchemaTest in EntitySchema causes the same effect (test failure first observed on Friday, though at the time I didn’t notice the effect on my database). Haven’t found a core test that triggers it yet, but I assume it’s not specific to Wikibase codebases.

Okay, tests/phpunit/includes/editpage/EditPageTest.php (in core) has the same effect as well.

1. Wikibase\Repo\Tests\Specials\SpecialNewItemTest::testAllNecessaryFormFieldsArePresent_WhenRendered

Wikimedia\Rdbms\DBQueryError: Error 1051: Unknown table 'wiki1.unittest_abuse_filter_action'
Function: CloneDatabase::cloneTableStructure
Query: DROP TABLE unittest_abuse_filter_action CASCADE

So far the error always mentions this particular table, by the way – although that could just be because it’s the first alphabetically (ABuse…). This still happens even if I stop loading the AbuseFilter extension. (I guess it just tries to clone every table, regardless of whether it’s registered by a loaded extension or not?)

This patch does not fix the issue – apparently the table cannot be dropped but nevertheless exists:

diff --git a/includes/db/CloneDatabase.php b/includes/db/CloneDatabase.php
index 43e5513b82..156f6f18d0 100644
--- a/includes/db/CloneDatabase.php
+++ b/includes/db/CloneDatabase.php
@@ -100,9 +100,11 @@ public function cloneTableStructure() {
 					throw new LogicException( "Not dropping new table, as '$newTableName'"
 						. " is name of both the old and the new table." );
 				}
-				$this->db->dropTable( $tbl, __METHOD__ );
-				wfDebug( __METHOD__ . " dropping {$newTableName}" );
-				// Dropping the oldTable because the prefix was changed
+				if ( $this->db->tableExists( $tbl, __METHOD__ ) ) {
+					$this->db->dropTable( $tbl, __METHOD__ );
+					wfDebug( __METHOD__ . " dropping {$newTableName}" );
+					// Dropping the oldTable because the prefix was changed
+				}
 			}
 
 			# Create new table

But that points me in the right direction: if I set PHPUNIT_USE_NORMAL_TABLES=false, then the test passes. Something seems to be messed up with temporary tables. (I wonder if it’s a MariaDB bug… apparently I installed a new version, 11.2.2-1 of mariadb, on 24 November, which is relatively recent.)

In T352695#9380071, @Lucas_Werkmeister_WMDE wrote:

1. Wikibase\Repo\Tests\Specials\SpecialNewItemTest::testAllNecessaryFormFieldsArePresent_WhenRendered

Wikimedia\Rdbms\DBQueryError: Error 1051: Unknown table 'wiki1.unittest_abuse_filter_action'
Function: CloneDatabase::cloneTableStructure
Query: DROP TABLE unittest_abuse_filter_action CASCADE

So far the error always mentions this particular table, by the way – although that could just be because it’s the first alphabetically (ABuse…).

It’s not quite the first table, by the way. Stepping through CloneDatabase::cloneTableStructure() with the debugger reveals that abuse_filter is first, and is successfully dropped. abuse_filter_action is then the second table, and fails.

I wonder if the abuse_filter drop somehow cascades through to abuse_filter_action, dropping it prematurely… but I don’t see any kind of foreign key relation between the two.

In T352695#9380162, @Lucas_Werkmeister_WMDE wrote:

(I wonder if it’s a MariaDB bug… apparently I installed a new version, 11.2.2-1 of mariadb, on 24 November, which is relatively recent.)

I can’t say for sure whether it’s a bug or not, but the error vanishes if I downgrade MariaDB (mariadb, mariadb-libs, mariadb-clients) to 11.1.3-1.

Tagging DBAs – not urgent, and I know you’re dealing with an incident at the moment, but when you have some time, I assume y’all know how best to report bugs upstream? And/or could perhaps help me investigate this further?

(Also, even if the changed MariaDB behavior turns out to be a bug, MediaWiki should still handle the error better. Falling back to the non-test DB should never happen. If we can’t set up the cloned DB, abort the tests, by exit() if necessary.)

In T352695#9380162, @Lucas_Werkmeister_WMDE wrote:

This patch does not fix the issue – apparently the table cannot be dropped but nevertheless exists:

diff --git a/includes/db/CloneDatabase.php b/includes/db/CloneDatabase.php
index 43e5513b82..156f6f18d0 100644
--- a/includes/db/CloneDatabase.php
+++ b/includes/db/CloneDatabase.php
@@ -100,9 +100,11 @@ public function cloneTableStructure() {
 					throw new LogicException( "Not dropping new table, as '$newTableName'"
 						. " is name of both the old and the new table." );
 				}
-				$this->db->dropTable( $tbl, __METHOD__ );
-				wfDebug( __METHOD__ . " dropping {$newTableName}" );
-				// Dropping the oldTable because the prefix was changed
+				if ( $this->db->tableExists( $tbl, __METHOD__ ) ) {
+					$this->db->dropTable( $tbl, __METHOD__ );
+					wfDebug( __METHOD__ . " dropping {$newTableName}" );
+					// Dropping the oldTable because the prefix was changed
+				}
 			}
 
 			# Create new table

It turns out this patch is completely redundant. Database::dropTable(), at the very beginning, checks tableExists().

But: Changes and Improvements in MariaDB 11.2 mentions:

Temporary tables are now displayed in the Information Schema TABLES Table, SHOW TABLES and SHOW TABLE STATUS (MDEV-12459)

So this could be the relevant change here? tableExists() previously never showed the temporary table, so MediaWiki never issued the DROP query; but now it shows the temporary table, and so MediaWiki sends a DROP for it?

Still don’t know why it can’t be dropped, though…

In T352695#9380186, @Lucas_Werkmeister_WMDE wrote:

In T352695#9380162, @Lucas_Werkmeister_WMDE wrote:

(I wonder if it’s a MariaDB bug… apparently I installed a new version, 11.2.2-1 of mariadb, on 24 November, which is relatively recent.)

I can’t say for sure whether it’s a bug or not, but the error vanishes if I downgrade MariaDB (mariadb, mariadb-libs, mariadb-clients) to 11.1.3-1.

Tagging DBAs – not urgent, and I know you’re dealing with an incident at the moment, but when you have some time, I assume y’all know how best to report bugs upstream? And/or could perhaps help me investigate this further?

MariaDB 11.1 is still incredibly new and I'd not recommend using it in production/serious environment.

If downgrading works, you can always file a bug report via jira.mariadb.org (you need to register for that).
If I were you, I'd try the same with 10.6 and see if you get the same behaviour

MariaDB appears to be… returning the just-created unittest_abuse_filter table as a result of SHOW TABLES LIKE 'unittest_abuse_filter_action';, i.e. when MediaWiki is checking whether abuse_filter_action needs to be deleted before it’s created? o_O

Okay, am I being stupid about how SQL works, or is this a MariaDB bug after all? Isolated example:

MariaDB [wiki1]> CREATE TEMPORARY TABLE T352695 (a INT NOT NULL PRIMARY KEY AUTO_INCREMENT);
Query OK, 0 rows affected (0.001 sec)

MariaDB [wiki1]> SHOW TABLES LIKE 'T352695abcde';
+--------------------------------+
| Tables_in_wiki1 (T352695abcde) |
+--------------------------------+
| T352695                        |
+--------------------------------+
1 row in set (0.000 sec)

Apparently a temporary table with the name _ can match any SHOW TABLES LIKE query.

MariaDB [wiki1]> CREATE TEMPORARY TABLE _ (a INT);
Query OK, 0 rows affected (0.001 sec)

MariaDB [wiki1]> SHOW TABLES LIKE 'T352695';
+---------------------------+
| Tables_in_wiki1 (T352695) |
+---------------------------+
| _                         |
+---------------------------+
1 row in set (0.001 sec)

@Lucas_Werkmeister_WMDE thanks for filing this. I see the same behavior when running extensions/DiscussionTools/tests/phpunit/ThreadItemStoreTest.php, I am on MariaDB 11.1.3.

In T352695#9384102, @kostajh wrote:

@Lucas_Werkmeister_WMDE thanks for filing this. I see the same behavior when running extensions/DiscussionTools/tests/phpunit/ThreadItemStoreTest.php, I am on MariaDB 11.1.3.

After downgrading to 10.6, I don't see this problem.

In T352695#9384041, @Lucas_Werkmeister_WMDE wrote:

Apparently a temporary table with the name _ can match any SHOW TABLES LIKE query.

MariaDB [wiki1]> CREATE TEMPORARY TABLE _ (a INT);
Query OK, 0 rows affected (0.001 sec)

MariaDB [wiki1]> SHOW TABLES LIKE 'T352695';
+---------------------------+
| Tables_in_wiki1 (T352695) |
+---------------------------+
| _                         |
+---------------------------+
1 row in set (0.001 sec)

That's a bug, SHOW TABLES like '_352695'; should show the table because _ is a wildcard for 1 character, but it shouldn't work the other way around, and looks scary, security-wise!!!!!

In T352695#9384102, @kostajh wrote:

@Lucas_Werkmeister_WMDE thanks for filing this. I see the same behavior when running extensions/DiscussionTools/tests/phpunit/ThreadItemStoreTest.php, I am on MariaDB 11.1.3.

Based on the official docs, temporary tables should be shown by SHOW TABLES only starting with maria-db 11.2: https://mariadb.com/kb/en/show-tables/

So this looks like an incomplete feature unintentionally enabled prematurely?

Could it be https://jira.mariadb.org/browse/MDEV-28334 ?

In T352695#9384102, @kostajh wrote:

@Lucas_Werkmeister_WMDE thanks for filing this. I see the same behavior when running extensions/DiscussionTools/tests/phpunit/ThreadItemStoreTest.php, I am on MariaDB 11.1.3.

Strange, for me MariaDB 11.1.3 is the version where the bug doesn’t happen yet…

In T352695#9384145, @jcrespo wrote:

That's a bug, SHOW TABLES like '_352695'; should show the table because _ is a wildcard for 1 character, but it shouldn't work the other way around, and looks scary, security-wise!!!!!

Thanks for confirming :)

In T352695#9384189, @jcrespo wrote:

Could it be https://jira.mariadb.org/browse/MDEV-28334 ?

That was the closest task I found on their bugtracker too, but it seems to be about a slightly different command / query, and was also supposed to be fixed in 11.2.1 (I’m on 11.2.2). I guess the next step would be to report a new task there… do you want to do it (better contacts) or should I?

I can also reproduce the issue with their official Docker images btw:

1. docker run --rm --env MARIADB_ROOT_PASSWORD=root --name=T352695 mariadb:latest
2. docker exec -it T352695 mariadb -uroot -proot
3. CREATE DATABASE db; USE db;
4. CREATE TEMPORARY TABLE a (a INT);
5. SHOW TABLES LIKE 'b';

It's up to you if you want to report it, if not I'll do it. Whatever is easier for you

In T352695#9386209, @Lucas_Werkmeister_WMDE wrote:

In T352695#9384102, @kostajh wrote:

@Lucas_Werkmeister_WMDE thanks for filing this. I see the same behavior when running extensions/DiscussionTools/tests/phpunit/ThreadItemStoreTest.php, I am on MariaDB 11.1.3.

Strange, for me MariaDB 11.1.3 is the version where the bug doesn’t happen yet…

In T352695#9384145, @jcrespo wrote:

That's a bug, SHOW TABLES like '_352695'; should show the table because _ is a wildcard for 1 character, but it shouldn't work the other way around, and looks scary, security-wise!!!!!

Thanks for confirming :)

In T352695#9384189, @jcrespo wrote:

Could it be https://jira.mariadb.org/browse/MDEV-28334 ?

That was the closest task I found on their bugtracker too, but it seems to be about a slightly different command / query, and was also supposed to be fixed in 11.2.1 (I’m on 11.2.2). I guess the next step would be to report a new task there… do you want to do it (better contacts) or should I?

I can also reproduce the issue with their official Docker images btw:

1. docker run --rm --env MARIADB_ROOT_PASSWORD=root --name=T352695 mariadb:latest
2. docker exec -it T352695 mariadb -uroot -proot
3. CREATE DATABASE db; USE db;
4. CREATE TEMPORARY TABLE a (a INT);
5. SHOW TABLES LIKE 'b';

If this is the exact way to reproduce (please confirm), it doesn't happen with 11.1

root@db1106.eqiad.wmnet[enwiki]> CREATE DATABASE db; USE db;
Query OK, 1 row affected (0.001 sec)

Database changed
root@db1106.eqiad.wmnet[db]> CREATE TEMPORARY TABLE a (a INT);
Query OK, 0 rows affected (0.001 sec)

root@db1106.eqiad.wmnet[db]> SHOW TABLES LIKE 'b';
Empty set (0.001 sec)

root@db1106.eqiad.wmnet[db]> select @@version;
+--------------------+
| @@version          |
+--------------------+
| 11.1.0-MariaDB-log |
+--------------------+
1 row in set (0.001 sec)

In T352695#9389166, @Marostegui wrote:

It's up to you if you want to report it, if not I'll do it. Whatever is easier for you

Alright, I’ll do that tomorrow (not enough time today).

In T352695#9389239, @Marostegui wrote:

If this is the exact way to reproduce (please confirm), it doesn't happen with 11.1

That should be it, yeah. I’ll try it with different Docker versions tomorrow.

Once you've reported it please paste the link, so I can ping some MariaDB people that might be interested in seeing this.

Created MDEV-32973.

The MariaDB issue can be worked around with something like:

diff --git a/includes/libs/rdbms/database/DatabaseMySQL.php b/includes/libs/rdbms/database/DatabaseMySQL.php
index 817f2a3242..3e2bb672d8 100644
--- a/includes/libs/rdbms/database/DatabaseMySQL.php
+++ b/includes/libs/rdbms/database/DatabaseMySQL.php
@@ -321,7 +321,16 @@ public function tableExists( $table, $fname = __METHOD__ ) {
 		$query = new Query( $sql, self::QUERY_IGNORE_DBO_TRX | self::QUERY_CHANGE_NONE, 'SHOW', $table );
 		$res = $this->query( $query, $fname );
 
-		return $res->numRows() > 0;
+		foreach ( $res as $row ) {
+			$returnedTableName = reset( $row );
+			if ( $returnedTableName === $tableName ) {
+				return true;
+			} else {
+				wfWarn( "Database returned mismatching name $returnedTableName for requested $tableName (query: $sql), T352695 / MDEV-32973?", 1, false );
+			}
+		}
+
+		return false;
 	}
 
 	/**

Note that the use of nonstandard false for the wfWarn “level” is necessary to avoid throwing an error during the tests, because if an error is thrown, we just get the same behavior as before. This is still something that needs to be fixed in MediaWiki as well (T352695#9380189) – errors during test DB setup should not result in the tests running against the real DB instead.

Change 981555 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@master] Only set $dbSetup if setupTestDB() ends without throwing

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

In T352695#9380189, @Lucas_Werkmeister_WMDE wrote:

(Also, even if the changed MariaDB behavior turns out to be a bug, MediaWiki should still handle the error better. Falling back to the non-test DB should never happen. If we can’t set up the cloned DB, abort the tests, by exit() if necessary.)

In T352695#9392740, @Lucas_Werkmeister_WMDE wrote:

Note that the use of nonstandard false for the wfWarn “level” is necessary to avoid throwing an error during the tests, because if an error is thrown, we just get the same behavior as before. This is still something that needs to be fixed in MediaWiki as well (T352695#9380189) – errors during test DB setup should not result in the tests running against the real DB instead.

Should be fixed with tests: Only set $dbSetup if setupTestDB() ends without throwing, I think; when I apply that and run the tests from the task description T352695#9380022, I get the same DBQueryError in all six tests, which makes sense (each of them tries and fails to repeat the setup now, then crashes without clobbering the real database).

Change 981555 merged by jenkins-bot:

[mediawiki/core@master] tests: Only set $dbSetup if setupTestDB() ends without throwing

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

I think we can close this? The MediaWiki side should be fixed, and the MariaDB side is being fixed as well but we don’t really have to wait for that imho.

+1

In T352695#9392740, @Lucas_Werkmeister_WMDE wrote:

The MariaDB issue can be worked around with something like:

diff --git a/includes/libs/rdbms/database/DatabaseMySQL.php b/includes/libs/rdbms/database/DatabaseMySQL.php
index 817f2a3242..3e2bb672d8 100644
--- a/includes/libs/rdbms/database/DatabaseMySQL.php
+++ b/includes/libs/rdbms/database/DatabaseMySQL.php
@@ -321,7 +321,16 @@ public function tableExists( $table, $fname = __METHOD__ ) {
 		$query = new Query( $sql, self::QUERY_IGNORE_DBO_TRX | self::QUERY_CHANGE_NONE, 'SHOW', $table );
 		$res = $this->query( $query, $fname );
 
-		return $res->numRows() > 0;
+		foreach ( $res as $row ) {
+			$returnedTableName = reset( $row );
+			if ( $returnedTableName === $tableName ) {
+				return true;
+			} else {
+				wfWarn( "Database returned mismatching name $returnedTableName for requested $tableName (query: $sql), T352695 / MDEV-32973?", 1, false );
+			}
+		}
+
+		return false;
 	}
 
 	/**

Note that the use of nonstandard false for the wfWarn “level” is necessary to avoid throwing an error during the tests, because if an error is thrown, we just get the same behavior as before. This is still something that needs to be fixed in MediaWiki as well (T352695#9380189) – errors during test DB setup should not result in the tests running against the real DB instead.

Hm, do we want to apply a patch like this to MediaWiki as well, to work around the specific MariaDB bug? (We could skip the nonstandard wfWarn level now.) But if they fix this issue quickly, I don’t think that’s really necessary tbh. (In general, it’s probably not the responsibility of our rdbms library to sanity-check the database implementation.)

In T352695#9399073, @Lucas_Werkmeister_WMDE wrote:

But if they fix this issue quickly, I don’t think that’s really necessary tbh.

Well, the proposed pull request for MariaDB still hasn’t been merged :S and as I just ran into this issue again while looking into T360435, I suppose we should at least backport the MediaWiki core fix.

Change #1017801 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@REL1_41] tests: Only set $dbSetup if setupTestDB() ends without throwing

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

Change #1017802 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@REL1_40] tests: Only set $dbSetup if setupTestDB() ends without throwing

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

Change #1017803 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@REL1_39] tests: Only set $dbSetup if setupTestDB() ends without throwing

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

Change #1017801 merged by jenkins-bot:

[mediawiki/core@REL1_41] tests: Only set $dbSetup if setupTestDB() ends without throwing

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

Change #1017803 merged by jenkins-bot:

[mediawiki/core@REL1_39] tests: Only set $dbSetup if setupTestDB() ends without throwing

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

Change #1017802 merged by jenkins-bot:

[mediawiki/core@REL1_40] tests: Only set $dbSetup if setupTestDB() ends without throwing

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