diff --git a/core/lib/Drupal/Core/Database/Query/Insert.php b/core/lib/Drupal/Core/Database/Query/Insert.php index cb2ee9d336..58bd8d1811 100644 --- a/core/lib/Drupal/Core/Database/Query/Insert.php +++ b/core/lib/Drupal/Core/Database/Query/Insert.php @@ -79,22 +79,24 @@ public function execute() { } $last_insert_id = 0; - - // Each insert happens in its own query in the degenerate case. However, - // we wrap it in a transaction so that it is atomic where possible. On many - // databases, such as SQLite, this is also a notable performance boost. - $transaction = $this->connection->startTransaction(); $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions); - try { + // Per https://en.wikipedia.org/wiki/Insert_%28SQL%29#Multirow_inserts, + // not all databases implement SQL-92's standard syntax for multi-row + // inserts. Therefore, in the degenerate case, execute a separate query + // for each row, all within a single transaction for atomicity and + // performance. + $transaction = $this->connection->startTransaction(); foreach ($this->insertValues as $insert_values) { $stmt->execute($insert_values, $this->queryOptions); $last_insert_id = $this->connection->lastInsertId(); } } catch (\Exception $e) { - // One of the INSERTs failed, rollback the whole batch. - $transaction->rollBack(); + if (isset($transaction)) { + // One of the INSERTs failed, rollback the whole batch. + $transaction->rollBack(); + } // Rethrow the exception for the calling code. throw $e; } diff --git a/core/lib/Drupal/Core/Database/database.api.php b/core/lib/Drupal/Core/Database/database.api.php index 3c3877d562..ab70410887 100644 --- a/core/lib/Drupal/Core/Database/database.api.php +++ b/core/lib/Drupal/Core/Database/database.api.php @@ -176,10 +176,11 @@ * @code * function my_transaction_function() { * $connection = \Drupal::database(); - * // The transaction opens here. - * $transaction = $connection->startTransaction(); * * try { + * // The transaction opens here. + * $transaction = $connection->startTransaction(); + * * $id = $connection->insert('example') * ->fields(array( * 'field1' => 'string', @@ -192,13 +193,19 @@ * return $id; * } * catch (Exception $e) { - * // Something went wrong somewhere, so roll back now. - * $transaction->rollBack(); + * // Something went wrong somewhere. If the exception was thrown during + * // startTransaction(), then $transaction is NULL and there's nothing to + * // roll back. If the exception was thrown after a transaction was + * // successfully started, then it must be rolled back. + * if (isset($transaction)) { + * $transaction->rollBack(); + * } + * * // Log the exception to watchdog. * watchdog_exception('type', $e); * } * - * // $transaction goes out of scope here. Unless the transaction was rolled + * // $transaction goes out of scope here. Unless the transaction was rolled * // back, it gets automatically committed here. * } * diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php index 289d2a9934..b592f8a79b 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php @@ -746,15 +746,17 @@ public function delete(array $entities) { return; } - $transaction = $this->database->startTransaction(); try { + $transaction = $this->database->startTransaction(); parent::delete($entities); // Ignore replica server temporarily. \Drupal::service('database.replica_kill_switch')->trigger(); } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception($this->entityTypeId, $e); throw new EntityStorageException($e->getMessage(), $e->getCode(), $e); } @@ -797,8 +799,8 @@ protected function doDeleteFieldItems($entities) { * {@inheritdoc} */ public function save(EntityInterface $entity) { - $transaction = $this->database->startTransaction(); try { + $transaction = $this->database->startTransaction(); $return = parent::save($entity); // Ignore replica server temporarily. @@ -806,7 +808,9 @@ public function save(EntityInterface $entity) { return $return; } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception($this->entityTypeId, $e); throw new EntityStorageException($e->getMessage(), $e->getCode(), $e); } @@ -816,8 +820,8 @@ public function save(EntityInterface $entity) { * {@inheritdoc} */ public function restore(EntityInterface $entity) { - $transaction = $this->database->startTransaction(); try { + $transaction = $this->database->startTransaction(); // Insert the entity data in the base and data tables only for default // revisions. /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */ @@ -853,7 +857,9 @@ public function restore(EntityInterface $entity) { \Drupal::service('database.replica_kill_switch')->trigger(); } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception($this->entityTypeId, $e); throw new EntityStorageException($e->getMessage(), $e->getCode(), $e); } diff --git a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php index 46e96c9a88..48adc9772a 100644 --- a/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php @@ -763,12 +763,13 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $ } } - if ($this->database->supportsTransactionalDDL()) { - // If the database supports transactional DDL, we can go ahead and rely - // on it. If not, we will have to rollback manually if something fails. - $transaction = $this->database->startTransaction(); - } try { + if ($this->database->supportsTransactionalDDL()) { + // If the database supports transactional DDL, we can go ahead and rely + // on it. If not, we will have to rollback manually if something fails. + $transaction = $this->database->startTransaction(); + } + // Copy the data from the base table. $this->database->insert($dedicated_table_name) ->from($this->getSelectQueryForFieldStorageDeletion($field_table_name, $shared_table_field_columns, $dedicated_table_field_columns)) @@ -788,8 +789,10 @@ public function onFieldStorageDefinitionDelete(FieldStorageDefinitionInterface $ } } catch (\Exception $e) { - if (isset($transaction)) { - $transaction->rollBack(); + if ($this->database->supportsTransactionalDDL()) { + if (isset($transaction)) { + $transaction->rollBack(); + } } else { // Delete the dedicated tables. @@ -1724,12 +1727,12 @@ protected function deleteSharedTableSchema(FieldStorageDefinitionInterface $stor protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $storage_definition, FieldStorageDefinitionInterface $original) { if (!$this->storage->countFieldData($original, TRUE)) { // There is no data. Re-create the tables completely. - if ($this->database->supportsTransactionalDDL()) { - // If the database supports transactional DDL, we can go ahead and rely - // on it. If not, we will have to rollback manually if something fails. - $transaction = $this->database->startTransaction(); - } try { + if ($this->database->supportsTransactionalDDL()) { + // If the database supports transactional DDL, we can go ahead and rely + // on it. If not, we will have to rollback manually if something fails. + $transaction = $this->database->startTransaction(); + } // Since there is no data we may be switching from a shared table schema // to a dedicated table schema, hence we should use the proper API. $this->performFieldSchemaOperation('delete', $original); @@ -1737,7 +1740,9 @@ protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $s } catch (\Exception $e) { if ($this->database->supportsTransactionalDDL()) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } } else { // Recreate tables. @@ -1815,12 +1820,12 @@ protected function updateDedicatedTableSchema(FieldStorageDefinitionInterface $s */ protected function updateSharedTableSchema(FieldStorageDefinitionInterface $storage_definition, FieldStorageDefinitionInterface $original) { if (!$this->storage->countFieldData($original, TRUE)) { - if ($this->database->supportsTransactionalDDL()) { - // If the database supports transactional DDL, we can go ahead and rely - // on it. If not, we will have to rollback manually if something fails. - $transaction = $this->database->startTransaction(); - } try { + if ($this->database->supportsTransactionalDDL()) { + // If the database supports transactional DDL, we can go ahead and rely + // on it. If not, we will have to rollback manually if something fails. + $transaction = $this->database->startTransaction(); + } // Since there is no data we may be switching from a dedicated table // to a schema table schema, hence we should use the proper API. $this->performFieldSchemaOperation('delete', $original); @@ -1828,7 +1833,9 @@ protected function updateSharedTableSchema(FieldStorageDefinitionInterface $stor } catch (\Exception $e) { if ($this->database->supportsTransactionalDDL()) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } } else { // Recreate original schema. diff --git a/core/lib/Drupal/Core/EventSubscriber/MenuRouterRebuildSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/MenuRouterRebuildSubscriber.php index b94aa9fa5b..7d0d64c0ad 100644 --- a/core/lib/Drupal/Core/EventSubscriber/MenuRouterRebuildSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/MenuRouterRebuildSubscriber.php @@ -76,15 +76,17 @@ public function onRouterRebuild($event) { */ protected function menuLinksRebuild() { if ($this->lock->acquire(__FUNCTION__)) { - $transaction = $this->connection->startTransaction(); try { + $transaction = $this->connection->startTransaction(); // Ensure the menu links are up to date. $this->menuLinkManager->rebuild(); // Ignore any database replicas temporarily. $this->replicaKillSwitch->trigger(); } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception('menu', $e); } diff --git a/core/lib/Drupal/Core/Menu/MenuTreeStorage.php b/core/lib/Drupal/Core/Menu/MenuTreeStorage.php index b0d8787b4a..cc471496e0 100644 --- a/core/lib/Drupal/Core/Menu/MenuTreeStorage.php +++ b/core/lib/Drupal/Core/Menu/MenuTreeStorage.php @@ -310,8 +310,8 @@ protected function doSave(array $link) { } } - $transaction = $this->connection->startTransaction(); try { + $transaction = $this->connection->startTransaction(); if (!$original) { // Generate a new mlid. // @todo Remove the 'return' option in Drupal 11. @@ -334,7 +334,9 @@ protected function doSave(array $link) { $this->updateParentalStatus($link); } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } throw $e; } return $affected_menus; diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 4a02fb0310..2757012f25 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -95,8 +95,8 @@ public function dump(array $options = []): string { // Delete any old records first, then insert the new ones. That avoids // stale data. The transaction makes it atomic to avoid unstable router // states due to random failures. - $transaction = $this->connection->startTransaction(); try { + $transaction = $this->connection->startTransaction(); // We don't use truncate, because it is not guaranteed to be transaction // safe. try { @@ -149,7 +149,9 @@ public function dump(array $options = []): string { } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception('Routing', $e); throw $e; } diff --git a/core/modules/sqlite/src/Driver/Database/sqlite/Connection.php b/core/modules/sqlite/src/Driver/Database/sqlite/Connection.php index dfb8f53c42..a46c67ff85 100644 --- a/core/modules/sqlite/src/Driver/Database/sqlite/Connection.php +++ b/core/modules/sqlite/src/Driver/Database/sqlite/Connection.php @@ -2,6 +2,7 @@ namespace Drupal\sqlite\Driver\Database\sqlite; +use Drupal\Core\Database\DatabaseExceptionWrapper; use Drupal\Core\Database\DatabaseNotFoundException; use Drupal\Core\Database\Connection as DatabaseConnection; use Drupal\Core\Database\StatementInterface; @@ -108,7 +109,7 @@ public static function open(array &$connection_options = []) { ]; try { - $pdo = new \PDO('sqlite:' . $connection_options['database'], '', '', $connection_options['pdo']); + $pdo = new PDOConnection('sqlite:' . $connection_options['database'], '', '', $connection_options['pdo']); } catch (\PDOException $e) { if ($e->getCode() == static::DATABASE_NOT_FOUND) { @@ -416,7 +417,15 @@ public function prepareStatement(string $query, array $options, bool $allow_row_ } public function nextId($existing_id = 0) { - $this->startTransaction(); + try { + $this->startTransaction(); + } + catch (\PDOException $e) { + // $this->exceptionHandler()->handleExecutionException() + // requires a $statement argument, so we cannot use that. + throw new DatabaseExceptionWrapper($e->getMessage(), 0, $e); + } + // We can safely use literal queries here instead of the slower query // builder because if a given database breaks here then it can simply // override nextId. However, this is unlikely as we deal with short strings diff --git a/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php b/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php index 229fd03656..bcbb6f2bdf 100644 --- a/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php +++ b/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php @@ -2,6 +2,7 @@ namespace Drupal\sqlite\Driver\Database\sqlite; +use Drupal\Core\Database\DatabaseExceptionWrapper; use Drupal\Core\Database\Query\Insert as QueryInsert; /** @@ -23,6 +24,9 @@ public function __construct(Connection $connection, string $table, array $option unset($this->queryOptions['return']); } + /** + * {@inheritdoc} + */ public function execute() { if (!$this->preExecute()) { return NULL; @@ -35,41 +39,64 @@ public function execute() { return $this->connection->query((string) $this, $this->fromQuery->getArguments(), $this->queryOptions); } - // We wrap the insert in a transaction so that it is atomic where possible. - // In SQLite, this is also a notable performance boost. - $transaction = $this->connection->startTransaction(); - + // If there are any fields in the query, execute normal INSERT statements. if (count($this->insertFields)) { - // Each insert happens in its own query. $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions); - foreach ($this->insertValues as $insert_values) { + + if (count($this->insertValues) === 1) { + // Inserting a single row does not require a transaction to be atomic, + // and executes faster without a transaction wrapper. + $insert_values = $this->insertValues[0]; try { $stmt->execute($insert_values, $this->queryOptions); } catch (\Exception $e) { - // One of the INSERTs failed, rollback the whole batch. - $transaction->rollBack(); $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, $insert_values, $this->queryOptions); } } + else { + // Inserting multiple rows requires a transaction to be atomic, and + // executes faster as a single transaction. + try { + $transaction = $this->connection->startTransaction(); + } + catch (\PDOException $e) { + // $this->connection->exceptionHandler()->handleExecutionException() + // requires a $statement argument, so we cannot use that. + throw new DatabaseExceptionWrapper($e->getMessage(), 0, $e); + } + foreach ($this->insertValues as $insert_values) { + try { + $stmt->execute($insert_values, $this->queryOptions); + } + catch (\Exception $e) { + // One of the INSERTs failed, rollback the whole batch. + $transaction->rollBack(); + $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, $insert_values, $this->queryOptions); + } + } + } // Re-initialize the values array so that we can re-use this query. $this->insertValues = []; } + // If there are no fields in the query, execute an INSERT statement that + // only populates default values. else { $stmt = $this->connection->prepareStatement("INSERT INTO {{$this->table}} DEFAULT VALUES", $this->queryOptions); try { $stmt->execute(NULL, $this->queryOptions); } catch (\Exception $e) { - $transaction->rollBack(); $this->connection->exceptionHandler()->handleExecutionException($e, $stmt, [], $this->queryOptions); } } - // Transaction commits here when $transaction looses scope. return $this->connection->lastInsertId(); } + /** + * {@inheritdoc} + */ public function __toString() { // Create a sanitized comment string to prepend to the query. $comments = $this->connection->makeComment($this->comments); diff --git a/core/modules/sqlite/src/Driver/Database/sqlite/PDOConnection.php b/core/modules/sqlite/src/Driver/Database/sqlite/PDOConnection.php new file mode 100644 index 0000000000..7f9c4e4d2b --- /dev/null +++ b/core/modules/sqlite/src/Driver/Database/sqlite/PDOConnection.php @@ -0,0 +1,53 @@ +exec('BEGIN IMMEDIATE TRANSACTION') !== FALSE; + } + + /** + * {@inheritdoc} + */ + public function commit(): bool { + return $this->exec('COMMIT') !== FALSE; + } + + /** + * {@inheritdoc} + */ + public function rollBack(): bool { + return $this->exec('ROLLBACK') !== FALSE; + } + +} diff --git a/core/modules/workspaces/src/WorkspaceAssociation.php b/core/modules/workspaces/src/WorkspaceAssociation.php index 2609183fa4..57b87224e1 100644 --- a/core/modules/workspaces/src/WorkspaceAssociation.php +++ b/core/modules/workspaces/src/WorkspaceAssociation.php @@ -69,8 +69,8 @@ public function trackEntity(RevisionableInterface $entity, WorkspaceInterface $w $tracked_revision_id = key($tracked[$entity->getEntityTypeId()]); } - $transaction = $this->database->startTransaction(); try { + $transaction = $this->database->startTransaction(); // Update all affected workspaces that were tracking the current revision. // This means they are inheriting content and should be updated. if ($tracked_revision_id) { @@ -110,7 +110,9 @@ public function trackEntity(RevisionableInterface $entity, WorkspaceInterface $w } } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception('workspaces', $e); throw $e; } diff --git a/core/modules/workspaces/src/WorkspaceMerger.php b/core/modules/workspaces/src/WorkspaceMerger.php index 31a73e4e56..afdabc755a 100644 --- a/core/modules/workspaces/src/WorkspaceMerger.php +++ b/core/modules/workspaces/src/WorkspaceMerger.php @@ -92,8 +92,8 @@ public function merge() { throw new WorkspaceConflictException(); } - $transaction = $this->database->startTransaction(); try { + $transaction = $this->database->startTransaction(); foreach ($this->getDifferringRevisionIdsOnSource() as $entity_type_id => $revision_difference) { $revisions_on_source = $this->entityTypeManager->getStorage($entity_type_id) ->loadMultipleRevisions(array_keys($revision_difference)); @@ -113,7 +113,9 @@ public function merge() { } } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception('workspaces', $e); throw $e; } diff --git a/core/modules/workspaces/src/WorkspacePublisher.php b/core/modules/workspaces/src/WorkspacePublisher.php index 1d336cfe1e..3626325756 100644 --- a/core/modules/workspaces/src/WorkspacePublisher.php +++ b/core/modules/workspaces/src/WorkspacePublisher.php @@ -87,8 +87,8 @@ public function publish() { throw new WorkspaceConflictException(); } - $transaction = $this->database->startTransaction(); try { + $transaction = $this->database->startTransaction(); // @todo Handle the publishing of a workspace with a batch operation in // https://www.drupal.org/node/2958752. $this->workspaceManager->executeOutsideWorkspace(function () { @@ -118,7 +118,9 @@ public function publish() { }); } catch (\Exception $e) { - $transaction->rollBack(); + if (isset($transaction)) { + $transaction->rollBack(); + } watchdog_exception('workspaces', $e); throw $e; }