Page MenuHomePhabricator

Database tables in PostgreSQL and Oracle are treated differently from other tables in other database. Why?
Closed, ResolvedPublic

Description

I am working on automated test with SemanticMediaWiki, I encountered "CREATE TABLE" error on duplication of table name. The immediate code in

https://github.com/wikimedia/mediawiki/blob/master/includes/db/CloneDatabase.php#L91-L102

Why?

Event Timeline

Yoonghm raised the priority of this task from to Medium.
Yoonghm updated the task description. (Show Details)
Yoonghm subscribed.

Hi @Aklapper,

I am not sure besides Semantic MediaWiki, which extension needs to use cloneTableStructure(). I performed automated tests, the process stopped due to

Error: 42P07 ERROR: relation "sunittest_smw_di_coords" already exists

I believe the error was caused by running multiple automated test jobs in one session.

From http://www.postgresql.org/docs/9.2/static/sql-createtable.html#AEN70273

Temporary tables are automatically dropped at the end of a session, or optionally at the end of the current 
transaction (see ON COMMIT below). Existing permanent tables with the same name are not visible to the 
current session while the temporary table exists, unless they are referenced with schema-qualified names. 
Any indexes created on a temporary table are automatically temporary as well.

In other words,

  • cloneTableStructure() should drop 'postgres' for the check as It is not allowed to have to same duplicated temporary table name in the same session. I have tested it in PostgreSQL 9.4 (I believe same thing holds for older versions). Same logic applies to Oracle.
  • PostgreSQL allows duplicated table name (as it is in pg_temp_%) as the public (default or in your search path) so the code does not really correct.
  • Temporarily tables will be deleted.

So the code should be fixed (at least with postgres):

	/**
	 * Clone the table structure
	 */
	public function cloneTableStructure() {
		global $wgSharedTables, $wgSharedDB;
		foreach ( $this->tablesToClone as $tbl ) {
			if ( $wgSharedDB && in_array( $tbl, $wgSharedTables, true ) ) {
				// Shared tables don't work properly when cloning due to
				// how prefixes are handled (bug 65654)
				throw new MWException( "Cannot clone shared table $tbl." );
			}
			# Clean up from previous aborted run.  So that table escaping
			# works correctly across DB engines, we need to change the pre-
			# fix back and forth so tableName() works right.

			self::changePrefix( $this->oldTablePrefix );
			$oldTableName = $this->db->tableName( $tbl, 'raw' );

			self::changePrefix( $this->newTablePrefix );
			$newTableName = $this->db->tableName( $tbl, 'raw' );

			if ( $this->dropCurrentTables
				&& !in_array( $this->db->getType(), array( 'oracle' ) )
			) {
				if ( $oldTableName === $newTableName ) {
					// Last ditch check to avoid data loss
					throw new MWException( "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}\n" );
				//Dropping the oldTable because the prefix was changed
			}

			# Create new table
			wfDebug( __METHOD__ . " duplicating $oldTableName to $newTableName\n" );
			$this->db->duplicateTableStructure( $oldTableName, $newTableName, $this->useTemporaryTables );
		}
	}

So the code should be fixed (at least with postgres):

Ah, thanks for taking a look!

You are very welcome to use developer access to submit this as a Git branch directly into Gerrit.
If you don't want to set up Git/Gerrit, you can also use the Gerrit Patch Uploader. Thanks again!

@Yoonghm: Awesome! Could you update your commit message as per https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines to summarize the change in the first line, plus link to this very Phabricator task? (You can edit direct in the web browser interface of Gerrit.)

@Yoonghm: If you're interested in this field, please take a look at T39702.

Change 241489 had a related patch set uploaded (by Nemo bis; owner: Yoonghm):
[mediawiki/core@master] Remove exception and drop cloned tables in PostgresSQL too

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

Change 241489 merged by jenkins-bot:
[mediawiki/core@master] Remove exception and drop cloned tables in PostgresSQL too

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

Krinkle assigned this task to aaron.
Krinkle moved this task from Untriaged to Rdbms library on the MediaWiki-libs-Rdbms board.
Krinkle edited projects, added Performance-Team; removed Patch-For-Review.