Field creation error needs correct cleanup

Patch work correctly and fix problem with DB unused tables. Need tests.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.moduleundefined
@@ -212,8 +212,31 @@ function _field_sql_storage_schema($field) {
-    db_create_table($name, $table);
+    try {
+        db_create_table($name, $table);
+        $created_tables[] = $name;
+      }
+      catch (Exception $e) {
+        // We have to remove already created tables.
+        if (Database::getConnection()->supportsTransactionalDDL()) {
+          $transaction->rollback();
+        }
+        else {
+          foreach ($created_tables as $name) {
+            db_drop_table($name);
+          }
+        }
+        throw $e;
+      }

Formatting indent issue.

+++ b/core/modules/field_ui/field_ui.testundefined
@@ -727,3 +727,80 @@ class FieldUIAlterTestCase extends DrupalWebTestCase {
+  function testCreateFieldFailureCleanup() {
+    // Create a test field.
+    $this->fieldUIAddNewFieldFail('admin/structure/types/manage/' . $this->hyphen_type);

I can't see reason to have separate method fieldUIAddNewIfledFail().

Here are attached rerolled patch with changes.

14 days to next Drupal core point release.

+++ b/core/modules/field_ui/field_ui.admin.inc
@@ -863,7 +863,8 @@ function field_ui_field_overview_form_submit($form, &$form_state) {
+    // It is true only if there was no exceptions.

there WERE no exceptions

Does this issue affects D7 as well?

@franz thank you for review. Yes, this issue affects D7 as well. Here is rerolled patch.

So I'll add the tag. Let's wait for test results now.

We introduced transactions for a reason, we should not try to manually rollback operations for databases that don't support it. Wrapping this in a transaction seems fine, but not the emulation.

This still a concern in D8? Unassigned issue too.

I guess it's still a concern in Drupal 8. I'm sure it's still a concern in Drupal 7.
We're facing a SchemaObjectExistsException 'Table @name already exists.'

Looking at the latest patch, and having that use case in mind, it doesn't make sense to (re)throw the exception - which says the table already exists - because the code just deleted that table. So, I suggest, if it's specifically that exception type, then don't throw $e. What do you think?

I also checked #1551132: When trying to create a table that already exists but is empty, recreate the table rather than throwing a DatabaseSchemaObjectExistsException but I like the approach from this issue more.

Forget my complaint against (re)throwing the exception. Looking at the patch once more made it clear to me that the code rolls back / drops the tables that got successfully created before the table that failed creating. Throwing an exception for that failure makes sense of course.

We need to test if this is still an issue on Drupal 9

I tested this on Drupal 10.1.x, standard install using the steps in the Issue Summary. I was able to reproduce the problem. The error I get when trying to add a field 'sample' to a page is
The website encountered an unexpected error. Please try again later.

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'field_sample_value' in 'where clause': SELECT 1 AS "expression" FROM "node_revision__field_sample" "t" WHERE "field_sample_value" IS NOT NULL LIMIT 1 OFFSET 0; Array ( ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->countFieldData() (line 1787 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

The patch needs to be updated, with #8 in mind.

How would one create this without modifying the database? How does this crop up, without doing something that 99% of site builders don't do?