Issue #1007830 by drunken monkey, Damien Tournoud, bfroehle: Fixed Nested transactions throw exceptions on ddl changes.

2 files changed, 109 insertions, 5 deletions

diff --git a/includes/database/mysql/database.inc b/includes/database/mysql/database.inc
index 262cc6051..47ef8d52e 100644
--- a/includes/database/mysql/database.inc
+++ b/includes/database/mysql/database.inc
@@ -133,6 +133,55 @@ class DatabaseConnection_mysql extends DatabaseConnection {
     catch (PDOException $e) {
     }
   }
+
+  /**
+   * Overridden to work around 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 (!PDO::commit()) {
+          throw new DatabaseTransactionCommitFailedException();
+        }
+      }
+      else {
+        // Attempt to release this savepoint in the standard way.
+        try {
+          $this->query('RELEASE SAVEPOINT ' . $name);
+        }
+        catch (PDOException $e) {
+          // However, in MySQL (InnoDB), savepoints are automatically committed
+          // when tables are altered or created (DDL transactions are not
+          // supported). This can cause exceptions due to trying to release
+          // savepoints which no longer exist.
+          //
+          // To avoid exceptions when no actual error has occurred, we silently
+          // succeed for PDOExceptions with error code 42000 ("Syntax error or
+          // access rule violation").
+          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 a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test
index c22d1fc5d..143640d60 100644
--- a/modules/simpletest/tests/database_test.test
+++ b/modules/simpletest/tests/database_test.test
@@ -3251,8 +3251,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();
@@ -3269,7 +3271,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.'));
 
@@ -3289,12 +3291,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
@@ -3315,6 +3317,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.
@@ -3396,6 +3414,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.'));
+    }
+  }
 }