Index: includes/database/database.inc
===================================================================
RCS file: /cvs/drupal/drupal/includes/database/database.inc,v
retrieving revision 1.40
diff -u -p -r1.40 database.inc
--- includes/database/database.inc	30 Dec 2008 20:27:24 -0000	1.40
+++ includes/database/database.inc	8 Jan 2009 04:13:56 -0000
@@ -116,6 +116,49 @@
  * 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. To start a new transaction,
+ * simply call $txn = db_transaction(): in your own code. 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.
+ *
+ * Example:
+ *
+ * @code
+ * function my_transaction_function() {
+ *   // The transaction opens here.
+ *   $txn = db_transaction();
+ *
+ *   $id = db_insert('example')
+ *     ->fields(array(
+ *       'field1' => 'mystring',
+ *       'field2' => 5,
+ *     ))
+ *     ->execute();
+ *
+ *   my_other_function($id);
+ *
+ *   return $id;
+ *   // $txn goes out of scope here, and the entire transaction commits.
+ * }
+ *
+ * function my_other_function($id) {
+ *   // The transaction is still open here.
+ *
+ *   if ($id % 2 == 0) {
+ *     db_update('example')
+ *       ->condition('id', $id)
+ *       ->fields(array('field2' => 10))
+ *       ->execute();
+ *   }
+ * }
+ * @endcode
+ *
  */
 
 
@@ -167,6 +210,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,
@@ -226,6 +287,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
@@ -706,6 +776,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
@@ -729,6 +809,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
@@ -792,9 +949,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;
   }
 
   /**
@@ -818,6 +990,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();
+  }
 }
 
 /**
@@ -1175,7 +1361,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 { }
 
 /**
  * Exception thrown for merge queries that do not make semantic sense.
@@ -1201,7 +1400,7 @@ class InvalidMergeQueryException extends
  * 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 {
 
@@ -1212,88 +1411,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();
   }
 
 }
@@ -1769,6 +1893,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	8 Jan 2009 04:13:56 -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.15
diff -u -p -r1.15 database.inc
--- includes/database/pgsql/database.inc	26 Dec 2008 11:48:18 -0000	1.15
+++ includes/database/pgsql/database.inc	8 Jan 2009 04:13:56 -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.34
diff -u -p -r1.34 database_test.test
--- modules/simpletest/tests/database_test.test	4 Jan 2009 19:56:51 -0000	1.34
+++ modules/simpletest/tests/database_test.test	8 Jan 2009 04:13:57 -0000
@@ -2113,3 +2113,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());
+    }
+  }
+}