This issue is part of #2157455: [Meta] Make Drupal 8 work with PostgreSQL or remove support from core before release.
Problem/Motivation (Original reported by @ingaro)
I did some investigation into why Drupal\migrate_drupal\Tests\d6\MigrateTaxonomyTermTest is failing.
Not all taxonomy terms are being migrated - the taxonomy_term_data table has just 4 rows on PostgreSQL, whereas there are 6 on MySQL. It looks like this is down to the order in which they are processed during the migration. If we try to migrate a term for which the parent term has not yet been migrated, the child term is skipped. The order seems to vary between MySQL and PostgreSQL.
To illustrate this, adding orderBy('tid') to the query in Drupal\migrate_drupal\Plugin\migrate\source\d6\Term means that the parent terms are loaded first, and the tests to pass. However that's only true in this particular case, so it isn't a real solution, but may help with diagnosis.
Proposed resolution
Fix remaining fails
Remaining tasks
Write patch.
User interface changes
None.
API changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2494415-term.patch | 759 bytes | bzrudi71 |
Comments
Comment #1
bzrudi71 commentedComment #2
bzrudi71 commented@ingaro, thanks for your investigation. As we do not use the logic of the entity system here, but doing some kind of manual queries, I think it's up to migrate to handle ordering. So I see nothing wrong with your idea of an orderBy() ensuring the order by tid, so let's just do it :) Patch attached.
Comment #3
bzrudi71 commentedThat should pass MySQL also I think, let's see
Comment #4
daffie commentedThe patch looks good to me.
The Drupal\migrate_drupal\Tests\d6\MigrateTaxonomyTermTest passes with the patch applied.
It gets a RTBC from me.
Comment #6
webchickSo many of these same types of patches. It would be wonderful if somehow our testing framework could "self-test" for this condition and raise an error before we commit the patches that break pgsql. :( A girl can dream. ;)
At any rate, committed and pushed to 8.0.x. Thanks!
Comment #7
tstoecklerProbably needs confirmation from someone else but I don't think this is the correct fix. It is not always a given that the term ID or a parent term is lower that its child's term ID. The query now assumes that, though. It makes the tests pass because for the test data this is actually the case, but it should not be generally assumed.
Comment #8
tstoecklerSince it is actually not more or less broken than before (just differently) I don't actually think it makes sense to revert this, because now at least this doesn't bother the PgSQL folks, but nevertheless we should properly fix the migration.
Comment #9
bzrudi71 commentedThanks @webchick for commit :) @tstoeckler I think your concerns are outside the scope of this patch and should better be addressed by the migrate folks by a follow-up? :)
Comment #12
daffie commentedAs far as I can see, there are no longer problems with the MigrateTaxonomyTermTest.