Preserve the backup tables after an entity type schema conversion process

As mentioned in #2984782: Add an API for converting entity type schemas to (non-)revisionable/(non-)translatable, with or without data, I'd be fine with a PHP setting that lets people enable the automatic deletion behavior. Sites with big databases would just need to enable it, but those are likely to have more tech-savy people behind them. OTOH non-technical site owners relying on dangerous YOLO update workflows directly on prod would have a safety net by default.

What about something like a Dashboard for the admin where pending actions must be approved? We could then add a notification there and when the admin approves it, then we could flag those tables for deletion. I think that we could use such a dashboard for a lot of different things instead of introducing more and more settings :).

I like the idea of using a PHP setting for this. It's very simple and definitely sufficient.

We just need to make a point of creating a change record for this to bring this new setting to people's attention.

This is exactly what I had in mind, thanks! Storing backup metadata in the key-value store opens up the door for all sort of fancy solutions on top of this, like the one that @hchonov was mentioning in #3. At the same we have a change that is small enough to still have a chance to go into 8.7.0.

The patch looks very good to me, however it seems that we have no test coverage for the case where the setting is FALSE. It would be good to add that as well, since that's a critical piece of this solution. I think we should really add that.

Quick code review:

1. +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
   @@ -428,11 +453,14 @@ public function onFieldableEntityTypeUpdate(EntityTypeInterface $entity_type, En
   +    $temporary_prefix = 'tmp_' . static::getTemporaryTableMappingPrefix($entity_type, $field_storage_definitions);
   

   The ::getTemporaryTableMappingPrefix() method was added in 8.7.x and is internal, however, since it was in -beta1, I think it would be better to add a new (protected) method to compute just the hash and invoke that from ::getTemporaryTableMappingPrefix(), so we do not need to change its behavior. This would have the additional (admittedly minor) benefit of keeping the tmp_ prefix encapsulated.

2. +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
   @@ -554,11 +582,22 @@ protected function postUpdateEntityTypeSchema(EntityTypeInterface $entity_type,
   +      $backup_key = $sandbox['backup_prefix_key'];
   

   I was wondering whether it would make sense to append the timestamp to the prefix key, just to be super safe :)

Looks good, thanks!

Added a release notes snippet.

Committed/pushed to 8.8.x and cherry-picked to 8.7.x.