Page MenuHomePhabricator

SQLPlatform::getDatabaseAndTableIdentifier returning null causes lots of logspam
Closed, ResolvedPublic

Description

https://integration.wikimedia.org/ci/job/quibble-for-mediawiki-core-vendor-mysql-php85/3/consoleFull

for example

13:59:58 595) Wikimedia\Tests\Rdbms\DatabaseTest::testShouldRejectPseudoPermanentTemporaryTableOperationsOnReplicaDatabaseConnection
13:59:58 Using null as an array offset is deprecated, use an empty string instead
13:59:58 
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:608
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:717
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:633
13:59:58 /workspace/src/tests/phpunit/includes/DB/DatabaseTestHelper.php:162
13:59:58 /workspace/src/tests/phpunit/unit/includes/libs/Rdbms/Database/DatabaseTest.php:813
13:59:58 
13:59:58 596) Wikimedia\Tests\Rdbms\DatabaseTest::testShouldAcceptWriteQueryOnPrimaryDatabaseConnection
13:59:58 Using null as an array offset is deprecated, use an empty string instead
13:59:58 
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:593
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:673
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:633
13:59:58 /workspace/src/tests/phpunit/includes/DB/DatabaseTestHelper.php:162
13:59:58 /workspace/src/tests/phpunit/unit/includes/libs/Rdbms/Database/DatabaseTest.php:835
13:59:58 
13:59:58 597) Wikimedia\Tests\Rdbms\DatabaseTest::testShouldRejectWriteQueryOnPrimaryDatabaseConnectionWhenReplicaQueryRoleFlagIsSet
13:59:58 Using null as an array offset is deprecated, use an empty string instead
13:59:58 
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:593
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:673
13:59:58 /workspace/src/includes/libs/Rdbms/Database/Database.php:633
13:59:58 /workspace/src/tests/phpunit/includes/DB/DatabaseTestHelper.php:162
13:59:58 /workspace/src/tests/phpunit/unit/includes/libs/Rdbms/Database/DatabaseTest.php:850

These seem related to the usage of $pt (the second value) from the return value of getDatabaseAndTableIdentifier being null

	/**
	 * Determine whether a write query affects a permanent table.
	 * This includes pseudo-permanent tables.
	 *
	 * @param Query $query
	 * @return bool
	 */
	private function hasPermanentTable( Query $query ) {
		if ( $query->getVerb() === 'CREATE TEMPORARY' ) {
			// Temporary table creation is allowed
			return false;
		}
		$table = $query->getWriteTable();
		if ( $table === null ) {
			// Parse error? Assume permanent.
			return true;
		}
		[ $db, $pt ] = $this->platform->getDatabaseAndTableIdentifier( $table );
		$tempInfo = $this->sessionTempTables[$db][$pt] ?? null;
		return !$tempInfo || $tempInfo->pseudoPermanent;
	}

It is documented to return null, but that now causes a lot of noise:

	/**
	 * Get the database identifer and prefixed table name identifier for a table
	 *
	 * The table name is assumed to be relative to the current DB domain
	 *
	 * This method is useful for TEMPORARY table tracking. In MySQL, temp tables with identical
	 * names can co-exist on different databases, which can be done via CREATE and USE. Note
	 * that SQLite/PostgreSQL do not allow changing the database within a session. This method
	 * omits the schema identifier for several reasons:
	 *   - MySQL/MariaDB do not support schemas at all.
	 *   - SQLite/PostgreSQL put all TEMPORARY tables in the same schema (TEMP and pgtemp,
	 *     respectively). When these engines resolve a table reference, they first check for
	 *     a matching table in the temp schema, before checking the current DB domain schema.
	 *     Note that this breaks table segregation based on the schema component of the DB
	 *     domain, e.g. a temp table with unqualified name "x" resolves to the same underlying
	 *     table whether the current DB domain is "my_db-schema1-mw_" or "my_db-schema2-mw_".
	 *     By ignoring the schema, we can at least account for this.
	 *   - Exposing the the TEMP/pg_temp schema here would be too leaky of an abstraction,
	 *     running the risk of unexpected results, such as identifiers that don't match. It is
	 *     easier to just avoid creating identically-named TEMPORARY tables on different schemas.
	 *
	 * @internal only to be used inside rdbms library
	 * @param string $table Table name
	 * @return array{0:string|null,1:string} (unquoted database name, unquoted prefixed table name)
	 */
	public function getDatabaseAndTableIdentifier( string $table ) {
		$components = $this->qualifiedTableComponents( $table );
		switch ( count( $components ) ) {
			case 1:
				return [ $this->currentDomain->getDatabase(), $components[0] ];
			case 2:
				return $components;
			default:
				throw new DBLanguageError( 'Too many table components' );
		}
	}

Event Timeline

Change #1227880 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@master] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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

Change #1227880 merged by jenkins-bot:

[mediawiki/core@master] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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

Change #1227942 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@REL1_45] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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

Change #1227954 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@REL1_44] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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

Change #1227955 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@REL1_43] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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

Change #1227942 merged by jenkins-bot:

[mediawiki/core@REL1_45] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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

Change #1227955 merged by jenkins-bot:

[mediawiki/core@REL1_43] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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

Change #1227954 merged by jenkins-bot:

[mediawiki/core@REL1_44] Rdbms: Get strings from SQLPlatform::getDatabaseAndTableIdentifier

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