Problem/Motivation

There were a few reports of failed updates from 8.6.x to 8.7.x where the underlying problem was that the UUID stored in the last installed field storage definitions didn't match the one from the live definitions for configurable fields.

Proposed resolution

Make unique storage identifier differences be reported as a missing/needed field storage update by the entity definition update manager.

While this change wouldn't have helped directly with the failed update, it would've made site admins aware that there is a problem with (some) fields that needs to be fixed.

Remaining tasks

Review.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

Release notes snippet

Nope.

CommentFileSizeAuthor
#19 3101598-19.patch2.83 KBkarishmaamin
#2 3101598.patch2.84 KBamateescu

Comments

amateescu created an issue. See original summary.

amateescu’s picture

amateescu commented
Status: Active » Needs review
StatusFileSize
new 3101598.patch2.84 KB
amateescu’s picture

amateescu commented
Issue tags: +Drupal 8 upgrade path
amateescu’s picture

amateescu commented
Title: DIfferring field storage identifiers should be reported as a missing field definition update » Differring field storage identifiers should be reported as a missing field storage definition update
Issue tags: +entity storage, +WI critical
daffie’s picture

daffie commented

While this change wouldn't have helped directly with the failed update, it would've made site admins aware that there is a problem with (some) fields that needs to be fixed.

How are site admin going to find out that there is a problem that they should fix? Do we need a change record for that? Can we as Drupal core developers do something to help them with that?

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented

How are site admin going to find out that there is a problem that they should fix? Do we need a change record for that? Can we as Drupal core developers do something to help them with that?

Unfortunately finding inconsistencies requires debugging to check what exactly is the problem ... It was always like that and we could provide better and exactly errors about where the inconsistency relies. For example when detecting the unique identifier inconsistency we could provide a message exactly about this. This however is more complicated when the storage schema changed itself as those are nested arrays and providing automatic error retrieval would not be easy and we might just wanna stick to saying that there is a mismatch. 

+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php
@@ -273,6 +273,7 @@ public function requiresFieldStorageSchemaChanges(FieldStorageDefinitionInterfac

@@ -273,6 +273,7 @@ public function requiresFieldStorageSchemaChanges(FieldStorageDefinitionInterfac
+      $storage_definition->getUniqueStorageIdentifier() != $original->getUniqueStorageIdentifier() ||
       $storage_definition->hasCustomStorage() != $original->hasCustomStorage() ||
       $storage_definition->getSchema() != $original->getSchema() ||
       $storage_definition->isRevisionable() != $original->isRevisionable() ||

The unique storage identifier does not affect the storage schema, does it? If it does not, then this is not the right place to report the inconsistency.

amateescu’s picture

amateescu commented

The unique storage identifier does not affect the storage schema, does it?

It does, because a different storage identifier can mean that there is a different field type using that field name, and a different field type usually means a different field (storage) schema.

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented

It does, because a different storage identifier can mean that there is a different field type using that field name, and a different field type usually means a different field (storage) schema.

Makes sense, but then the following check should be failing right? :
$storage_definition->getSchema() != $original->getSchema()

amateescu’s picture

amateescu commented

Right. The problem that this patch tries to alleviate is when someone has the same field name and type, but with different storage identifiers (regardless of how that could happen). That situation can lead to broken entity type schema updates, because the temporary table names created by that process are based on the unique field storage identifiers.

hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Status: Needs review » Needs work

Ok, let's then document this in the code where the condition is being added as it isn't that obvious.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

amateescu’s picture

amateescu commented
Issue tags: -WI critical

Not sure why I tagged this issue with 'WI critical' when I opened it, but at this time it's not really blocking Workspaces from being marked as stable, so untagging.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue tags: +Needs reroll, +Needs issue summary update, +Bug Smash Initiative
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

FWIW https://github.com/larowlan/schema_diff can be used to unpick schema differences

karishmaamin’s picture

karishmaamin commented
Status: Needs work » Needs review
StatusFileSize
new 3101598-19.patch2.83 KB

Re-rolled patch against 9.5.x. Please review

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

For the issue summary update per #18

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.