diff --git includes/database/database.inc includes/database/database.inc index 4a3cbb4..e036a83 100644 --- includes/database/database.inc +++ includes/database/database.inc @@ -194,7 +194,7 @@ abstract class DatabaseConnection extends PDO { /** * The key representing this connection. - * + * * The key is a unique string which identifies a database connection. A * connection can be a single server or a cluster of master and slaves (use * target to pick between master and slave). @@ -1019,12 +1019,12 @@ abstract class DatabaseConnection extends PDO { } // Commit everything since SAVEPOINT $name. - while($savepoint = array_pop($this->transactionLayers)) { + while ($savepoint = array_pop($this->transactionLayers)) { if ($savepoint != $name) continue; // If there are no more layers left then we should commit. if (empty($this->transactionLayers)) { - if (!parent::commit()) { + if (!$this->commitTransaction()) { throw new DatabaseTransactionCommitFailedException(); } } @@ -1170,12 +1170,23 @@ abstract class DatabaseConnection extends PDO { * @throws DatabaseTransactionExplicitCommitNotAllowedException * * @see DatabaseTransaction + * @see DatabaseConnection::commitTransaction() */ public function commit() { throw new DatabaseTransactionExplicitCommitNotAllowedException(); } /** + * Allows the database driver to directly commit a transaction. + * + * @see PDO::commit() + * @see DatabaseConnection::commit() + */ + protected function commitTransaction() { + return parent::commit(); + } + + /** * Retrieves an unique id from a given sequence. * * Use this function if for some reason you can't use a serial field. For diff --git includes/database/mysql/database.inc includes/database/mysql/database.inc index 262cc60..22389ad 100644 --- includes/database/mysql/database.inc +++ includes/database/mysql/database.inc @@ -133,6 +133,48 @@ class DatabaseConnection_mysql extends DatabaseConnection { catch (PDOException $e) { } } + + /** + * Overridden to work-a-round issues to mysql not supporting transactional DDL. + */ + public function popTransaction($name) { + if (!$this->supportsTransactions()) { + return; + } + if (!$this->inTransaction()) { + throw new DatabaseTransactionNoActiveException(); + } + + // Commit everything since SAVEPOINT $name. + while ($savepoint = array_pop($this->transactionLayers)) { + if ($savepoint != $name) continue; + + // If there are no more layers left then we should commit. + if (empty($this->transactionLayers)) { + if (!$this->commitTransaction()) { + throw new DatabaseTransactionCommitFailedException(); + } + } + else { + // As MySQL does not support transactional DDL previous queries may have + // implicitly committed pending transactions. To avoid exceptions when + // no actual error has occurred, we silently succeed for PDOExceptions + // with error code 42000 ("Syntax error or access rule violation"). + try { + $this->query('RELEASE SAVEPOINT ' . $name); + } + catch (PDOException $e) { + if ($e->getCode() != '42000') { + throw $e; + } + // If one SAVEPOINT was released automatically, then all were. + // Therefore, we keep just the topmost transaction. + $this->transactionLayers = array('drupal_transaction'); + } + break; + } + } + } } diff --git modules/simpletest/tests/database_test.test modules/simpletest/tests/database_test.test index 2a086ce..7d2442a 100644 --- modules/simpletest/tests/database_test.test +++ modules/simpletest/tests/database_test.test @@ -3215,8 +3215,10 @@ class DatabaseTransactionTestCase extends DatabaseTestCase { * Suffix to add to field values to differentiate tests. * @param $rollback * Whether or not to try rolling back the transaction when we're done. + * @param $ddl_statement + * Whether to execute a DDL statement during the inner transaction. */ - protected function transactionOuterLayer($suffix, $rollback = FALSE) { + protected function transactionOuterLayer($suffix, $rollback = FALSE, $ddl_statement = FALSE) { $connection = Database::getConnection(); $depth = $connection->transactionDepth(); $txn = db_transaction(); @@ -3233,7 +3235,7 @@ class DatabaseTransactionTestCase extends DatabaseTestCase { // We're already in a transaction, but we call ->transactionInnerLayer // to nest another transaction inside the current one. - $this->transactionInnerLayer($suffix, $rollback); + $this->transactionInnerLayer($suffix, $rollback, $ddl_statement); $this->assertTrue($connection->inTransaction(), t('In transaction after calling nested transaction.')); @@ -3253,12 +3255,12 @@ class DatabaseTransactionTestCase extends DatabaseTestCase { * Suffix to add to field values to differentiate tests. * @param $rollback * Whether or not to try rolling back the transaction when we're done. + * @param $ddl_statement + * Whether to execute a DDL statement during the transaction. */ - protected function transactionInnerLayer($suffix, $rollback = FALSE) { + protected function transactionInnerLayer($suffix, $rollback = FALSE, $ddl_statement = FALSE) { $connection = Database::getConnection(); - $this->assertTrue($connection->inTransaction(), t('In transaction in nested transaction.')); - $depth = $connection->transactionDepth(); // Start a transaction. If we're being called from ->transactionOuterLayer, // then we're already in a transaction. Normally, that would make starting @@ -3279,6 +3281,22 @@ class DatabaseTransactionTestCase extends DatabaseTestCase { $this->assertTrue($connection->inTransaction(), t('In transaction inside nested transaction.')); + if ($ddl_statement) { + $table = array( + 'fields' => array( + 'id' => array( + 'type' => 'serial', + 'unsigned' => TRUE, + 'not null' => TRUE, + ), + ), + 'primary key' => array('id'), + ); + db_create_table('database_test_1', $table); + + $this->assertTrue($connection->inTransaction(), t('In transaction inside nested transaction.')); + } + if ($rollback) { // Roll back the transaction, if requested. // This rollback should propagate to the last savepoint. @@ -3360,6 +3378,43 @@ class DatabaseTransactionTestCase extends DatabaseTestCase { $this->fail($e->getMessage()); } } + + /** + * Test the compatibility of transactions with DDL statements. + */ + function testTransactionWithDdlStatement() { + // First, test that a commit works normally, even with DDL statements. + try { + $this->transactionOuterLayer('D', FALSE, TRUE); + + // Because we committed, the inserted rows should both be present. + $saved_age = db_query('SELECT age FROM {test} WHERE name = :name', array(':name' => 'DavidD'))->fetchField(); + $this->assertIdentical($saved_age, '24', t('Can retrieve DavidD row after commit.')); + $saved_age = db_query('SELECT age FROM {test} WHERE name = :name', array(':name' => 'DanielD'))->fetchField(); + $this->assertIdentical($saved_age, '19', t('Can retrieve DanielD row after commit.')); + // The created table should also exist. + $count = db_query('SELECT COUNT(id) FROM {database_test_1}')->fetchField(); + $this->assertIdentical($count, '0', t('Table was successfully created inside a transaction.')); + } + catch (Exception $e) { + $this->fail($e->getMessage()); + } + + // If we rollback the transaction, an exception might be thrown. + try { + $this->transactionOuterLayer('E', TRUE, TRUE); + + // Because we rolled back, the inserted rows shouldn't be present. + $saved_age = db_query('SELECT age FROM {test} WHERE name = :name', array(':name' => 'DavidE'))->fetchField(); + $this->assertNotIdentical($saved_age, '24', t('Cannot retrieve DavidE row after rollback.')); + $saved_age = db_query('SELECT age FROM {test} WHERE name = :name', array(':name' => 'DanielE'))->fetchField(); + $this->assertNotIdentical($saved_age, '19', t('Cannot retrieve DanielE row after rollback.')); + } + catch (Exception $e) { + // An exception also lets the test pass. + $this->assertTrue(true, t('Exception thrown on rollback after a DDL statement was executed.')); + } + } }