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.

CommentFileSizeAuthor
#2 2494415-term.patch759 bytesbzrudi71

Comments

bzrudi71’s picture

Title: PostgreSQL: Drupal\migrate_drupal\Tests\d6\MigrateTaxonomyTermTest » PostgreSQL: Fix migrate_drupal\Tests\d6\MigrateTaxonomyTermTest
bzrudi71’s picture

StatusFileSize
new759 bytes

@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.

bzrudi71’s picture

Status: Active » Needs review

That should pass MySQL also I think, let's see

daffie’s picture

Status: Needs review » Reviewed & tested by the community

The patch looks good to me.
The Drupal\migrate_drupal\Tests\d6\MigrateTaxonomyTermTest passes with the patch applied.
It gets a RTBC from me.

  • webchick committed 3cf64bf on 8.0.x
    Issue #2494415 by bzrudi71: PostgreSQL: Fix migrate_drupal\Tests\d6\...
webchick’s picture

Status: Reviewed & tested by the community » Fixed

So 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!

tstoeckler’s picture

Status: Fixed » Needs work

Probably 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.

tstoeckler’s picture

Since 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.

bzrudi71’s picture

Thanks @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? :)

  • webchick committed 3cf64bf on 8.1.x
    Issue #2494415 by bzrudi71: PostgreSQL: Fix migrate_drupal\Tests\d6\...

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

daffie’s picture

Status: Needs work » Closed (cannot reproduce)

As far as I can see, there are no longer problems with the MigrateTaxonomyTermTest.