diff -u b/core/modules/sqlite/src/Driver/Database/sqlite/Connection.php b/core/modules/sqlite/src/Driver/Database/sqlite/Connection.php --- b/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; @@ -416,7 +417,15 @@ } public function nextId($existing_id = 0) { - $this->startTransaction(); + try { + $this->startTransaction(); + } + catch (\PDOException $e) { + // $this->connection->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 -u b/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php b/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php --- b/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php +++ b/core/modules/sqlite/src/Driver/Database/sqlite/Insert.php @@ -44,7 +44,8 @@ $stmt = $this->connection->prepareStatement((string) $this, $this->queryOptions); if (count($this->insertValues) === 1) { - // A single row can be inserted without the overhead of a transaction. + // 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); @@ -54,11 +55,14 @@ } } else { - // Inserting multiple rows requires a transaction to be atomic. + // 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) { only in patch2: unchanged: --- 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 multirow + // 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; } only in patch2: unchanged: --- 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. * } * only in patch2: unchanged: --- 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. only in patch2: unchanged: --- 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;