Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Over at #2721313: Upgrade path between revisionable / non-revisionable entities, we're trying to add support for an upgrade path for making entity types revisionable when they already contain data. This means we have to add a NOT NULL field (reivision_id) to a table with existing rows, with initial values that match an existing field, the entity ID.
Proposed resolution
Support a 'initial_from_field' field schema specification key which will automatically populate the added field with values from the specified existing field.
Remaining tasks
Review.
User interface changes
Nope.
API changes
API addition, the new 'initial_from_field' field schema spec key.
Data model changes
Nope.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | interdiff.txt | 1.38 KB | amateescu |
| #4 | 2753475-4.patch | 5.47 KB | amateescu |
Comments
Comment #2
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedHere's what I had in mind.
Comment #3
timmillwoodSome history of the issue in #2746279: Adding not null fields to table which have data, which has been close in favour of this issue.
Comment #4
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedI expected the Postgres failure, let's not try to compare integer columns with string columns :) Also fixed the SQLite code.
Comment #5
dawehnerLooks okay for me.
Comment #6
timmillwoodAwesome job @amateescu!
Even without the postgres and sqlite fix #2721313: Upgrade path between revisionable / non-revisionable entities shows that this solves the issue it sets out to.
Comment #7
daffie CreditAttribution: daffie commentedIn #2746279: Adding not null fields to table which have data there is a test with
DESCRIBE {table}for MySQL. There are other methods for PostgreSQL (SELECT * FROM INFORMATION_SCHEMA.COLUMNS WHERE table_name = 'table') and SQLite (PRAGMA table_info(table)) to do the same. I think that they are needed here.Comment #8
daffie CreditAttribution: daffie commentedWe are now testing a "serial_column". Can we add testing for regular not null columns?
Comment #9
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented@daffie, the key that we're adding here is not standard SQL behavior, so I don't see how testing any kind of SQL introspection is useful.
Also, the fact that we're testing a "serial" column also doesn't matter, we just need to test against a column that has data in it.
Comment #10
daffie CreditAttribution: daffie commented@amateescu: You are right. I did not look good enough at the patch.
Comment #11
dawehnerI still think some of the database maintainer should have a look at it, because well, technically for itself this is a new feature of the DB subsystem.
Comment #12
Crell CreditAttribution: Crell at Platform.sh commentedI can't tell from the patch itself, is this something that works on table creation, or just on table update? That possible inconsistency is my only concern.
Comment #13
timmillwood@Crell - From what I understand this is on table update only, which is by design. The problem we're trying to solve is if you're adding a
NOT NULLcolumn to a table which already has data we need to set an initial value before making it not null. When creating a new table there is no data so you can addNOT NULLwith no issues.Comment #14
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThis works just like the existing 'initial' schema key, which means that it works on both table creation and table updates, it's just that on table creation there are no initial values set because there is no other column with data to copy from.
\Drupal\KernelTests\Core\Database\SchemaTest::assertFieldAdditionRemoval()tests a field spec for both new and existing tables :)Comment #15
alexpottThis looks good but we should have a change record to announce the support for this and also search docs on d.o in case we need to update stuff - there might be nothing see https://www.drupal.org/developing/api/schema
Comment #16
alexpottComment #17
timmillwoodChange record added: https://www.drupal.org/node/2755201
Comment #18
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedUpdated the CR a bit to provide a more clear usage example :)
Comment #19
alexpottCommitted bae3e31 and pushed to 8.2.x. Thanks!