Skip to content

Loading…

DDC-1337: Rollback doesn't work on transactional multi table update with mysql #1953

Closed
doctrinebot opened this Issue · 12 comments

2 participants

@doctrinebot

Jira issue originally created by user gedrox:

When doing transactional multi table update using DQL query the changes are not rolled back after exception is raised after the update.

```html$em->transactional(function($entityManager) {
$dql = "UPDATE ..."; // some multi table update DQL
$query = $entityManager->createQuery($dql);
$query->execute();

throw new Exception();

});```

This is because Doctrine executes "DROP TABLE" for temporary table created for the update but MySQL is doing commit right after DROP and CREATE statements automatically.

From PHP documentation:

bq. "Some databases, including MySQL, automatically issue an implicit COMMIT when a database definition language (DDL) statement such as DROP TABLE or CREATE TABLE is issued within a transaction. The implicit COMMIT will prevent you from rolling back any other changes within the transaction boundary."

@doctrinebot

Comment created by gedrox:

I have done temporary workaround just by commenting out the line

165: $conn->executeUpdate($this->_dropTempTableSql);

from class Doctrine\ORM\Query\Exec\MultiTableUpdateExecutor.

I guess it is not correct solution for all platforms, is it?

@doctrinebot

Comment created by @beberlei:

This is a problem with MySQL and Inheritance. Not sure how we can fix this.

@doctrinebot

Comment created by gedrox:

Partial workaround is not dropping the temporary tables at all when on MySQL (at least) platform. These tables are session based and will be dropped automatically.

Still it can't be predicted when a temporary table will be needed inside the transaction, so we cannot make sure the CREATE TEMPORARY TABLE won't be called inside transaction after something has been updated already.

I think the best solution would be to raise exception when temporary table must be created or dropped while inside the transaction. Developer should call temporary table creation and removal manually before/after the transactional code part if it might be needed by the Doctrine.

In my opinion this is really important issue because no warning or exception was raised by the code in the case of rollback, still it didn't work as expected.

@doctrinebot

Comment created by gedrox:

Additionally needed to change temporary table name generation function ClassMetadataInfo::getTemporaryIdTableName() by adding incrementing property suffix:

return str*replace('.', '_', $this->table['name'] . '_id_tmp*' . $this->counter<ins></ins>);

after removed temporary table drop execution.

@doctrinebot

Comment created by @beberlei:

This seems very tricky:

  • A solution would mean: Open a new connection, create a "real" table. Use that from the original connection.
  • Another would be to create the temporary table BEFORE starting the transaction. However how do we know this?
  • An immediate and simple fix would be to throw an exception and prevent UPDATE and DELETE on joined table inheritance entities when run inside an transaction.
@doctrinebot

Comment created by @beberlei:

I wouldnt consider the php documentation to be a good source on MySQL.

If you read http://dev.mysql.com/doc/refman/5.1/en/implicit-commit.html

You get:

{quote}
ALTER TABLE, CREATE TABLE, and DROP TABLE do not commit a transaction if the TEMPORARY keyword is used. (This does not apply to other operations on temporary tables such as CREATE INDEX, which do cause a commit.) However, although no implicit commit occurs, neither can the statement be rolled back. Therefore, use of such statements will violate transaction atomicity: For example, if you use CREATE TEMPORARY TABLE and then roll back the transaction, the table remains in existence.
{quote}

@doctrinebot

Comment created by @beberlei:

Verified that transactions do not get committed when you CREATE or DROP temporary tables. That is also with MySQL 5.1.54 / Ubuntu

You seem to have some other kind of error or something?

@doctrinebot

Comment created by gedrox:

I'm almost sure that updated data was committed in my situation, but will check and provide the test case if possible.

@doctrinebot

Comment created by gedrox:

Found the problem!
It's because the Doctrine calls "DROP TABLE" instead of "DROP TEMPORARY TABLE" when getting rid of the temporary tables.

To prove this strange MySQL behaviour, such PHP script can be executed:

$pdo = new PDO('mysql:dbname=test', 
        'root', 
        'root', 
        array(PDO::ATTR*ERRMODE => PDO::ERRMODE*EXCEPTION));

$pdo->exec('DROP TABLE IF EXISTS a');
$pdo->exec('CREATE TABLE a (a INT) ENGINE = innodb');

$pdo->beginTransaction();

$pdo->exec('INSERT INTO a VALUES (1)');
$pdo->exec('CREATE TEMPORARY TABLE c (b int) ENGINE = innodb');
$pdo->exec('DROP TABLE c');
$pdo->exec('INSERT INTO a VALUES (2)');

$pdo->rollBack();

$st = $pdo->prepare('SELECT * FROM a');
$st->execute();
$row = $st->fetch(PDO::FETCH_ASSOC);

var_dump($row);

The dump should output "false", still the first inserted record is output.

This is actual at least on Mysql 5.1.54-1ubuntu4 and PHP 5.3.6.

The solution is trivial - use "DROP TEMPORARY TABLE" syntax instead.

@doctrinebot

Comment created by @beberlei:

Verified, even on mysql console client.

Thanks for investigating, big thumbs up.

@doctrinebot

Comment created by @beberlei:

Fixed.

@doctrinebot

Issue was closed with resolution "Fixed"

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot added this to the 2.1.2 milestone
@doctrinebot doctrinebot closed this
@doctrinebot doctrinebot added the Bug label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.