When a migration defines an entity reference and therefor depends upon another 2nd migration which gets run at a later time (thus not enforced via "migration_dependencies: required: ...") the migration will fail if the 2nd migration has Substr as one of the used plugins.
When the migration runs the entity references are created as stubs. The stubs are created creating a Row with only the target_id filled out, the rest of the values are empty. This stub row is then processed by the 2nd migration. When the 2nd migration defines Substr as the plugin for one of the values the Substr plugin will have to process a NULL value and throw MigrateException('The input value must be a string.');
This background-run of the 2nd migration on a stub row to create the entity-reference in the 1st migration effectively fails the whole migration.
The fix is to check for null on $value. If it is NULL then just return $value (NULL) because there won't be anything for Substr to do on NULL.
Comment | File | Size | Author |
---|---|---|---|
#20 | migration_process_null_empty-2800279-20.patch | 3.07 KB | iarla |
#12 | migration_process_null_empty-2800279-12.patch | 5.47 KB | agileadam |
#4 | substr_fails_stub_migration-2800279-4.patch | 1.69 KB | Jeroen |
#2 | substr_fails_stub_migration-2800279-2.patch | 913 bytes | Jeroen |
Comments
Comment #2
Jeroen CreditAttribution: Jeroen at Wunder commentedComment #3
Jeroen CreditAttribution: Jeroen at Wunder commentedComment #4
Jeroen CreditAttribution: Jeroen at Wunder commentedComment #5
Jeroen CreditAttribution: Jeroen at Wunder commentedI've added a test too.
Comment #6
benjy CreditAttribution: benjy at PreviousNext commentedIs this because the values are null in the source data? I'm not opposed to adding your check to the substr plugin but I think it could also be solved with:
Comment #7
mikeryanSorry Jeroen, now that I look closely at other string-manipulating process plugins such as explode and machine_name, I'm not sure the special NULL handling is appropriate here. The throwing of MigrateException is our established pattern in the !is_string() case - I agree it's harsh to hard-code throwing out the whole row for a missing value in one field (I think it would be better to return NULL and anyone who wanted to throw it out could chain to skip_on_empty with method: row), but at this point changing that behavior would be a backwards-compatibility break. And even if we thought it was worth a BC break, it's something that should be applied to all the process plugins, not just singling out this one. As benjy points out, you do have the option to skip_on_empty upstream.
Your stub problem is more general than substr - basically, in any scenario where stubbing may occur, the migration creating the stub must be prepared for empty values in all fields. I could have sworn we had at some point discussed adding a mechanism for providing default source values for stubs via the migration process plugin, but can't find such an issue for it at the moment. Unless/until we did something like that, I think about all we can do is document that you need to bullet-proof any migrations which are used for stubbing.
Comment #8
mikeryanComment #11
agileadamI just ran into this issue related to
substr
. Unfortunately it cannot be solved by simply adding askip_if_empty
plugin beforesubstr
in the chain. If you useskip_if_empty
and you are re-running a migration (--update
and/ortrack_changes: true
) and a field that previous had values now no longer has values, the field will be ignored during the migration process and the old field data will be kept in the entity. I can confirm that adding the if null check solves this issue.It happens with the StaticMap process plugin as well. I'm about to go see if an issues exists for that one.
Comment #12
agileadamHere's a patch (against 8.3.7) that adds NULL handling and associated tests to Concat, Substr, and StaticMap. I've tested on about 30 migrations (100k records) without issues (FWIW). I will create a patch against 8.4.x-dev if/when time allows.
Comment #17
mpp CreditAttribution: mpp as a volunteer and at AmeXio for District09 commentedI can confirm that the patch provides a workaround.
Otoh adding a skip process plugin also works.
@mikeryan, could you comment on #11?
Comment #20
iarla CreditAttribution: iarla at iFactory commentedRe-rolling #12 to account for Drupal 9.3.x code changes.
Comment #23
danflanagan8The Migrate Conditions module provides some tricks to help guard against problems during stubbing, specifically using the
is_stub
condition.See https://www.drupal.org/docs/contributed-modules/migrate-conditions/migra...