Index: includes/database/database.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/database/database.inc,v
retrieving revision 1.36
diff -u -p -r1.36 database.inc
--- includes/database/database.inc	24 Dec 2008 09:53:40 -0000	1.36
+++ includes/database/database.inc	25 Dec 2008 05:52:34 -0000
@@ -115,6 +115,51 @@
  * This method allows databases that need special data type handling to do so,
  * while also allowing optimizations such as multi-insert queries. UPDATE and
  * DELETE queries have a similar pattern.
+ *
+ *
+ * Drupal also supports transactions, including a transparent fallback for
+ * databases that do not support transactions.  However, transactions can get
+ * quite complicated when you try and start two transactions at the same time.
+ * The behavior in that case also varies between databases.
+ *
+ * A similar problem exists with nesting locks in C/C++. If the code has already
+ * acquired lock A and attempts to acquire lock A, the code will deadlock. If you
+ * write code that checks if it already has the lock and doesn't attempt to
+ * acquire it again, you avoid the deadlock but could release the lock
+ * prematurely.
+ *
+ * In SQL, we have the same problem. If your code is already in a transaction,
+ * starting a new transaction has the surprising and unfortunate consequence of
+ * committing the current transaction and starting a new one.
+ *
+ * Java solves the nesting problem with its locks by implementing support for a nesting
+ * structure similar to what we test below. Java allows you to mark functions as
+ * "synchronized," which causes the function to wait for lock acquisition before
+ * running and release the lock when no longer needed. If one synchronized
+ * function calls another in the same class, Java track the lock nesting. The
+ * outer function acquires the lock, the inner function performs no locking
+ * operations, and the outer function releases the lock when it returns.
+ *
+ * While we cannot declare functions "transactional" in PHP, we can emulate Java's
+ * nesting logic by using objects with constructors and destructors. A function
+ * simply calls "$txn = db_transaction();" as its first (or nearly first) operation
+ * to make itself transactional. If one transactional function calls another,
+ * our transaction abstraction layer nests them by performing no transactional
+ * operations (as far as the database sees) within the inner nesting layers.
+ *
+ * To start a new transaction, simply call:
+ *
+ * @code
+ * $txn = db_transaction();
+ * @endcode
+ *
+ * The transaction will remain open for as long as the variable $txn remains
+ * in scope.  When $txn is destroyed, the transaction will be committed.  If
+ * your transaction is nested inside of another then Drupal will track each
+ * transaction and only commit the outer-most transaction when the last
+ * transaction object goes out out of scope, that is, all relevant queries
+ * completed successfully.
+ *
  */
 
 
@@ -166,6 +211,24 @@ abstract class DatabaseConnection extend
   protected $preparedStatements = array();
 
   /**
+   * Track the number of "layers" of transactions currently active.
+   *
+   * On many databases transactions cannot nest.  Instead, we track
+   * nested calls to transactions and collapse them into a single
+   * transaction.
+   *
+   * @var int
+   */
+  protected $transactionLayers = 0;
+
+  /**
+   * Whether or not the active transaction (if any) will be rolled back.
+   *
+   * @var boolean
+   */
+  protected $willRollBack;
+
+  /**
    * The name of the Select class for this connection.
    *
    * Normally this and the following class names would be static variables,
@@ -225,6 +288,15 @@ abstract class DatabaseConnection extend
   protected $transactionSupport = TRUE;
 
   /**
+   * Whether this database connection supports transactional DDL.
+   *
+   * Set to FALSE by default because few databases support this feature.
+   *
+   * @var bool
+   */
+  protected $transactionalDDLSupport = FALSE;
+
+  /**
    * The schema object for this connection.
    *
    * @var object
@@ -698,6 +770,16 @@ abstract class DatabaseConnection extend
   }
 
   /**
+   * Determine if there is an active transaction open.
+   *
+   * @return
+   *   TRUE if we're currently in a transaction, FALSE otherwise.
+   */
+  public function inTransaction() {
+    return ($this->transactionLayers > 0);
+  }
+
+  /**
    * Returns a new DatabaseTransaction object on this connection.
    *
    * @param $required
@@ -721,6 +803,83 @@ abstract class DatabaseConnection extend
   }
 
   /**
+   * Schedule the current transaction for rollback.
+   *
+   * This method throws an exception if no transaction is active.
+   */
+  public function rollBack() {
+    if ($this->transactionLayers == 0) {
+      throw new NoActiveTransactionException();
+    }
+
+    $this->willRollBack = TRUE;
+  }
+
+  /**
+   * Determine if this transaction will roll back.
+   *
+   * Use this function to skip further operations if the current transaction
+   * is already scheduled to roll back. Throws an exception if no transaction
+   * is active.
+   *
+   * @return
+   *   TRUE if the transaction will roll back, FALSE otherwise.
+   */
+  public function willRollBack() {
+    if ($this->transactionLayers == 0) {
+      throw new NoActiveTransactionException();
+    }
+
+    return $this->willRollBack;
+  }
+
+  /**
+   * Increases the depth of transaction nesting.
+   *
+   * If no transaction is already active, we begin a new transaction.
+   *
+   * @see DatabaseTransaction
+   */
+  public function pushTransaction() {
+    ++$this->transactionLayers;
+
+    if ($this->transactionLayers == 1) {
+      if ($this->supportsTransactions()) {
+        parent::startTransaction();
+      }
+
+      // Reset any scheduled rollback
+      $this->willRollBack = FALSE;
+    }
+  }
+
+  /**
+   * Decreases the depth of transaction nesting, committing or rolling back if necessary.
+   *
+   * If we pop off the last transaction layer, then we either commit or roll back
+   * the transaction as necessary.  If no transaction is active, we throw
+   * an exception.
+   *
+   * @see DatabaseTransaction
+   */
+  public function popTransaction() {
+    if ($this->transactionLayers == 0) {
+      throw new NoActiveTransactionException();
+    }
+
+    --$this->transactionLayers;
+
+    if ($this->transactionLayers == 0 && $this->supportsTransactions()) {
+      if ($this->willRollBack) {
+        parent::rollBack();
+      }
+      else {
+        parent::commit();
+      }
+    }
+  }
+
+  /**
    * Runs a limited-range query on this database object.
    *
    * Use this as a substitute for ->query() when a subset of the query is to be
@@ -784,9 +943,24 @@ abstract class DatabaseConnection extend
 
   /**
    * Determine if this driver supports transactions.
+   *
+   * @return
+   *   TRUE if this connection supports transactions, FALSE otherwise.
    */
   public function supportsTransactions() {
-   return $this->transactionSupport;
+    return $this->transactionSupport;
+  }
+
+  /**
+   * Determine if this driver supports transactional DDL.
+   *
+   * DDL queries are those that change the schema, such as ALTER queries.
+   *
+   * @return
+   *   TRUE if this connection supports transactions for DDL queries, FALSE otherwise.
+   */
+  public function supportsTransactionalDDL() {
+    return $this->transactionalDDLSupport;
   }
 
   /**
@@ -810,6 +984,20 @@ abstract class DatabaseConnection extend
    *   The extra handling directives for the specified operator, or NULL.
    */
   abstract public function mapConditionOperator($operator);
+
+  /**
+   * Throws an exception to deny direct access to transaction commits.
+   *
+   * We do not want to allow users to commit transactions at any time, only
+   * by destroying the transaction object or allowing it to go out of scope.
+   * A direct commit bypasses all of the safety checks we've built on top of
+   * PDO's transaction routines.
+   *
+   * @see DatabaseTransaction
+   */
+  public function commit() {
+    throw new ExplicitTransactionsNotSupportedException();
+  }
 }
 
 /**
@@ -1167,7 +1355,20 @@ abstract class Database {
  * in use does not support transactions. The calling code must then take
  * appropriate action.
  */
-class TransactionsNotSupportedException extends PDOException { }
+class TransactionsNotSupportedException extends Exception { }
+
+/**
+ * Exception to throw when popTransaction() is called when no transaction is active.
+ */
+class NoActiveTransactionException extends Exception { }
+
+/**
+ * Exception to deny attempts to explicitly manage transactions.
+ *
+ * This exception will be thrown when the PDO connection commit() is called.
+ * Code should never call this method directly.
+ */
+class ExplicitTransactionsNotSupportedException extends Exception { }
 
 /**
  * A wrapper class for creating and managing database transactions.
@@ -1185,7 +1386,7 @@ class TransactionsNotSupportedException 
  * is that rollbacks won't actually do anything.
  *
  * In the vast majority of cases, you should not instantiate this class directly.
- * Instead, call ->startTransaction() from the appropriate connection object.
+ * Instead, call ->startTransaction(), from the appropriate connection object.
  */
 class DatabaseTransaction {
 
@@ -1196,88 +1397,13 @@ class DatabaseTransaction {
    */
   protected $connection;
 
-  /**
-   * Whether or not this connection supports transactions.
-   *
-   * This can be derived from the connection itself with a method call,
-   * but is cached here for performance.
-   *
-   * @var boolean
-   */
-  protected $supportsTransactions;
-
-  /**
-   * Whether or not this transaction has been rolled back.
-   *
-   * @var boolean
-   */
-  protected $hasRolledBack = FALSE;
-
-  /**
-   * Whether or not this transaction has been committed.
-   *
-   * @var boolean
-   */
-  protected $hasCommitted = FALSE;
-
-  /**
-   * Track the number of "layers" of transactions currently active.
-   *
-   * On many databases transactions cannot nest. Instead, we track
-   * nested calls to transactions and collapse them into a single
-   * transaction.
-   *
-   * @var int
-   */
-  protected static $layers = 0;
-
-  public function __construct(DatabaseConnection $connection) {
-    $this->connection = $connection;
-    $this->supportsTransactions = $connection->supportsTransactions();
-
-    if (self::$layers == 0 && $this->supportsTransactions) {
-      $connection->beginTransaction();
-    }
-
-    ++self::$layers;
-  }
-
-  /**
-   * Commit this transaction.
-   */
-  public function commit() {
-    --self::$layers;
-    if (self::$layers == 0 && $this->supportsTransactions) {
-      $this->connection->commit();
-      $this->hasCommitted = TRUE;
-    }
-  }
-
-  /**
-   * Roll back this transaction.
-   */
-  public function rollBack() {
-    if ($this->supportsTransactions) {
-      $this->connection->rollBack();
-      $this->hasRolledBack = TRUE;
-    }
-  }
-
-  /**
-   * Determine if this transaction has already been rolled back.
-   *
-   * @return
-   *   TRUE if the transaction has been rolled back, FALSE otherwise.
-   */
-  public function hasRolledBack() {
-    return $this->hasRolledBack;
+  public function __construct(DatabaseConnection &$connection) {
+    $this->connection = &$connection;
+    $this->connection->pushTransaction();
   }
 
   public function __destruct() {
-    --self::$layers;
-    if (self::$layers == 0 && $this->supportsTransactions && !$this->hasRolledBack && !$this->hasCommitted) {
-      $this->connection->commit();
-    }
+    $this->connection->popTransaction();
   }
 
 }
@@ -1753,6 +1879,26 @@ function db_select($table, $alias = NULL
 }
 
 /**
+ * Returns a new transaction object for the active database.
+ *
+ * @param $required
+ *   TRUE if the calling code will not function properly without transaction
+ *   support.  If set to TRUE and the active database does not support transactions
+ *   a TransactionsNotSupportedException exception will be thrown.
+ * @param $options
+ *   An array of options to control how the transaction operates.  Only the
+ *   target key has any meaning in this case.
+ * @return
+ *   A new DatabaseTransaction object for this connection.
+ */
+function db_transaction($required = FALSE, Array $options = array()) {
+  if (empty($options['target'])) {
+    $options['target'] = 'default';
+  }
+  return Database::getActiveConnection($options['target'])->startTransaction($required);
+}
+
+/**
  * Sets a new active database.
  *
  * @param $key
Index: includes/database/mysql/database.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/database/mysql/database.inc,v
retrieving revision 1.13
diff -u -p -r1.13 database.inc
--- includes/database/mysql/database.inc	24 Dec 2008 10:21:31 -0000	1.13
+++ includes/database/mysql/database.inc	25 Dec 2008 05:52:34 -0000
@@ -16,6 +16,9 @@ class DatabaseConnection_mysql extends D
   public function __construct(array $connection_options = array()) {
     // This driver defaults to non transaction support.
     $this->transactionSupport = !empty($connection_option['transactions']);
+    
+    // MySQL never supports transactional DDL.
+    $this->transactionalDDLSupport = FALSE;
 
     // Default to TCP connection on port 3306.
     if (empty($connection_options['port'])) {
Index: includes/database/pgsql/database.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/database/pgsql/database.inc,v
retrieving revision 1.14
diff -u -p -r1.14 database.inc
--- includes/database/pgsql/database.inc	20 Dec 2008 18:24:34 -0000	1.14
+++ includes/database/pgsql/database.inc	25 Dec 2008 05:52:34 -0000
@@ -16,6 +16,10 @@ class DatabaseConnection_pgsql extends D
   public function __construct(array $connection_options = array()) {
     // This driver defaults to transaction support, except if explicitly passed FALSE.
     $this->transactionSupport = !isset($connection_options['transactions']) || $connection_options['transactions'] === FALSE;
+    
+    // Transactional DDL is always available in PostgreSQL,
+    // but we'll only enable it if standard transactions are.
+    $this->transactionalDDLSupport = $this->transactionSupport;
 
     // Default to TCP connection on port 5432.
     if (empty($connection_options['port'])) {
Index: modules/simpletest/tests/database_test.test
===================================================================
RCS file: /cvs/drupal/drupal/modules/simpletest/tests/database_test.test,v
retrieving revision 1.30
diff -u -p -r1.30 database_test.test
--- modules/simpletest/tests/database_test.test	24 Dec 2008 10:21:32 -0000	1.30
+++ modules/simpletest/tests/database_test.test	25 Dec 2008 05:52:35 -0000
@@ -2093,3 +2093,227 @@ class DatabaseQueryTestCase extends Data
     $this->assertEqual(count($names), 3, t('Correct number of names returned'));
   }
 }
+
+/**
+ * Test transaction support, particularly nesting.
+ *
+ * We test nesting by having two transaction layers, an outer and inner. The
+ * outer layer encapsulates the inner layer. Our transaction nesting abstraction
+ * should allow the outer layer function to call any function it wants,
+ * especially the inner layer that starts its own transaction, and be
+ * confident that, when the function it calls returns, its own transaction
+ * is still "alive."
+ *
+ * Call structure:
+ *   transactionOuterLayer()
+ *     Start transaction
+ *     transactionInnerLayer()
+ *       Start transaction (does nothing in database)
+ *       [Maybe decide to roll back]
+ *     Do more stuff
+ *     Should still be in transaction A
+ *
+ */
+class DatabaseTransactionTestCase extends DatabaseTestCase {
+
+  function getInfo() {
+    return array(
+      'name' => t('Transaction tests'),
+      'description' => t('Test the transaction abstraction system.'),
+      'group' => t('Database'),
+    );
+  }
+
+  /**
+   * Helper method for transaction unit test. This "outer layer" transaction
+   * starts and then encapsulates the "inner layer" transaction. This nesting
+   * is used to evaluate whether the the database transaction API properly
+   * supports nesting. By "properly supports," we mean the outer transaction
+   * continues to exist regardless of what functions are called and whether
+   * those functions start their own transactions.
+   *
+   * In contrast, a typical database would commit the outer transaction, start
+   * a new transaction for the inner layer, commit the inner layer transaction,
+   * and then be confused when the outer layer transaction tries to commit its
+   * transaction (which was already committed when the inner transaction
+   * started).
+   *
+   * @param $suffix
+   *   Suffix to add to field values to differentiate tests.
+   * @param $rollback
+   *   Whether or not to try rolling back the transaction when we're done.
+   */
+  protected function transactionOuterLayer($suffix, $rollback = FALSE) {
+    $connection = Database::getActiveConnection();
+    $txn = db_transaction();
+
+    // Insert a single row into the testing table.
+    db_insert('test')
+      ->fields(array(
+        'name' => 'David' . $suffix,
+        'age' => '24',
+      ))
+      ->execute();
+
+    $this->assertTrue($connection->inTransaction(), t('In transaction before calling nested transaction.'));
+
+    // We're already in a transaction, but we call ->transactionInnerLayer
+    // to nest another transaction inside the current one.
+    $this->transactionInnerLayer($suffix, $rollback);
+
+    $this->assertTrue($connection->inTransaction(), t('In transaction after calling nested transaction.'));
+  }
+
+  /**
+   * Helper method for transaction unit tests. This "inner layer" transaction
+   * is either used alone or nested inside of the "outer layer" transaction.
+   *
+   * @param $suffix
+   *   Suffix to add to field values to differentiate tests.
+   * @param $rollback
+   *   Whether or not to try rolling back the transaction when we're done.
+   */
+  protected function transactionInnerLayer($suffix, $rollback = FALSE) {
+    $connection = Database::getActiveConnection();
+
+    // Start a transaction. If we're being called from ->transactionOuterLayer,
+    // then we're already in a transaction. Normally, that would make starting
+    // a transaction here dangerous, but the database API handles this problem
+    // for us by tracking the nesting and avoiding the danger.
+    $txn = db_transaction();
+
+    // Insert a single row into the testing table.
+    db_insert('test')
+      ->fields(array(
+        'name' => 'Daniel' . $suffix,
+        'age' => '19',
+      ))
+      ->execute();
+
+    $this->assertTrue($connection->inTransaction(), t('In transaction inside nested transaction.'));
+
+    if ($rollback) {
+      // Roll back the transaction, if requested.
+      // This rollback should propagate to the the outer transaction, if present.
+      $connection->rollBack();
+      $this->assertTrue($connection->willRollBack(), t('Transaction is scheduled to roll back after calling rollBack().'));
+    }
+  }
+
+  /**
+   * Test that a database that claims to support transactions will return a transaction object.
+   *
+   * If the active connection does not support transactions, this test does nothing.
+   */
+  function testTransactionsSupported() {
+    try {
+      $connection = Database::getActiveConnection();
+      if ($connection->supportsTransactions()) {
+
+        // Start a "required" transaction. This should fail if we do
+        // this on a database that does not actually support transactions.
+        $txn = db_transaction(TRUE);
+      }
+      $this->pass('Transaction started successfully.');
+    }
+    catch (TransactionsNotSupportedException $e) {
+      $this->fail("Exception thrown when it shouldn't have been.");
+    }
+  }
+
+  /**
+   * Test that a database that doesn't support transactions fails correctly.
+   *
+   * If the active connection supports transactions, this test does nothing.
+   */
+  function testTransactionsNotSupported() {
+    try {
+      $connection = Database::getActiveConnection();
+      if (!$connection->supportsTransactions()) {
+
+        // Start a "required" transaction. This should fail if we do this
+        // on a database that does not actually support transactions, and
+        // the current database does claim to NOT support transactions.
+        $txn = db_transaction(TRUE);
+      }
+      $this->fail('No transaction failure registered.');
+    }
+    catch (TransactionsNotSupportedException $e) {
+      $this->pass('Exception thrown for unsupported transactions.');
+    }
+  }
+
+  /**
+   * Test transaction rollback on a database that supports transactions.
+   *
+   * If the active connection does not support transactions, this test does nothing.
+   */
+  function testTransactionRollBackSupported() {
+    // This test won't work right if transactions are not supported.
+    if (!Database::getActiveConnection()->supportsTransactions()) {
+      return;
+    }
+    try {
+      // Create two nested transactions. Roll back from the inner one.
+      $this->transactionOuterLayer('B', TRUE);
+
+      // Neither of the rows we inserted in the two transaction layers
+      // should be present in the tables post-rollback.
+      $saved_age = db_query("SELECT age FROM {test} WHERE name = :name", array(':name' => 'DavidB'))->fetchField();
+      $this->assertNotIdentical($saved_age, '24', t('Cannot retrieve DavidB row after commit.'));
+      $saved_age = db_query("SELECT age FROM {test} WHERE name = :name", array(':name' => 'DanielB'))->fetchField();
+      $this->assertNotIdentical($saved_age, '19', t('Cannot retrieve DanielB row after commit.'));
+    }
+    catch(Exception $e) {
+      $this->fail($e->getMessage());
+    }
+  }
+
+  /**
+   * Test transaction rollback on a database that does not support transactions.
+   *
+   * If the active driver supports transactions, this test does nothing.
+   */
+  function testTransactionRollBackNotSupported() {
+    // This test won't work right if transactions are supported.
+    if (Database::getActiveConnection()->supportsTransactions()) {
+      return;
+    }
+    try {
+      // Create two nested transactions. Attempt to roll back from the inner one.
+      $this->transactionOuterLayer('B', TRUE);
+
+      // Because our current database claims to not support transactions,
+      // the inserted rows should be present despite the attempt to roll back.
+      $saved_age = db_query("SELECT age FROM {test} WHERE name = :name", array(':name' => 'DavidB'))->fetchField();
+      $this->assertIdentical($saved_age, '24', t('DavidB not rolled back, since transactions are not supported.'));
+      $saved_age = db_query("SELECT age FROM {test} WHERE name = :name", array(':name' => 'DanielB'))->fetchField();
+      $this->assertIdentical($saved_age, '19', t('DanielB not rolled back, since transactions are not supported.'));
+    }
+    catch(Exception $e) {
+      $this->fail($e->getMessage());
+    }
+  }
+
+  /**
+   * Test committed transaction.
+   *
+   * The behavior of this test should be identical for connections that support
+   * transactions and those that do not.
+   */
+  function testCommittedTransaction() {
+    try {
+      // Create two nested transactions. The changes should be committed.
+      $this->transactionOuterLayer('A');
+
+      // Because we committed, both of the inserted rows should be present.
+      $saved_age = db_query("SELECT age FROM {test} WHERE name = :name", array(':name' => 'DavidA'))->fetchField();
+      $this->assertIdentical($saved_age, '24', t('Can retrieve DavidA row after commit.'));
+      $saved_age = db_query("SELECT age FROM {test} WHERE name = :name", array(':name' => 'DanielA'))->fetchField();
+      $this->assertIdentical($saved_age, '19', t('Can retrieve DanielA row after commit.'));
+    }
+    catch(Exception $e) {
+      $this->fail($e->getMessage());
+    }
+  }
+}