Problem/Motivation

Currently when you have a revisionable entity type with a non-revisionable field storage definition requiring a dedicated table, the (default) table mapping returns no dedicated revision table, while the storage and storage schema handlers expect the revision table to be there and try to read from/write to it.

This is bad, because the more we start relying on the table mapping to write schema-agnostic code, the more this discrepancy will bite us. Here's a prime example: #3056539: Updating an entity type from non-revisionable to revisionable fails if it has non-revisionable fields stored in dedicated tables.

Proposed resolution

Use the same logic across the table mapping and the SQL storage/schema. Since reconciling the two behaviors is a BC break in either direction, we will need a BC layer and some form of deprecation.

We need to figure out whether we want the table mapping to behave as the SQL storage/schema or viceversa. Given that it’s perfectly legal to have a non-revisionable field on a revisionable entity type and that in such case there is absolutely no point in having a revision table, since it would be a straight copy of the data table, it's probably more correct to extend the table mapping logic to the SQL schema, however this is likely to require a more complex BC layer, as people might be relying on the existence of revision tables. In this case we'd likely be deprecating the existence of revision field tables for non-revisionable fields on revisionable entity types without actually dropping them.

Remaining tasks

  • Decide how to reconcile the two behaviors
  • Write a patch
  • Reviews

User interface changes

None

API changes

TBD

Data model changes

TBD

Release notes snippet

TBD

CommentFileSizeAuthor
#2 3113639-2.patch8.75 KBplach

Comments

plach created an issue. See original summary.

plach’s picture

Status: Active » Needs review
StatusFileSize
new8.75 KB

Here's a start at making things work the table mapping way.

plach’s picture

plach’s picture

Title: The default table mapping and the SQL storage do not agree on whether non-revisionable fields should get a revision table » The default table mapping and the SQL storage do not agree on whether non-revisionable bundle fields should get a revision table
Issue summary: View changes

More accurate title/IS

Status: Needs review » Needs work

The last submitted patch, 2: 3113639-2.patch, failed testing. View results

catch’s picture

however this is likely to require a more complex BC layer, as people might be relying on the existence of revision tables. In this case we'd likely be deprecating the existence of revision field tables for non-revisionable fields on revisionable entity types without actually dropping them.

In terms of the bc layer, seems there are two ways to inform people, trying to think through what it might look like:

1. A hook_requirements() that detects fields with the wrong schema, and warns the site administrator.

2. A deprecation error triggered by the bc layer itself.

We could then link to instructions informing people how to fix the schema via a hook_update_N() for one field at a time, or to mark the field as revisionable in the field definition if that's what they wanted after all.

Then when we want to remove that bc layer, seems like there are two options:

1. A hook_requirements('update') that prevents sites from updating unless they either update the schema, or set the field to revisionable.

2. A hook_update_N() that changes the schema for any fields it finds.

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.

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.

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.

acbramley’s picture

Popped up in BSI triage, it seems like this is still an issue based on the code in HEAD. Would be good to have some steps to reproduce this bug.

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.