Migrate translations for D6 i18n taxonomy 'localized' terms

@jhodgdon: The other issue introduces a d6_taxonomy_term_translation migration, that should be migrating the terms. Did that migration run when you tested it? If so, did it generate any errors?

@mikeryan: the translation migrations in #2784371: Migrate D6 i18n taxonomy term translation (but not yet localized translations) were done to account for explicit language info on taxonomy terms, which is one of the modes i18n can work. However when translations are directly on the term ("localizations" in i18n lingo) that is not yet supported.

Updated d6_taxonomy_term_translation.yml and the test. The migration needs translations: true on the destination. And then added a migration_lookup to the tid, renamed migration to migration_lookup. And finally skip the row if the language is empty. In the test fixture the language field only has data when the term has been translated.

Yes, please test again.

I'm trying to figure out the next steps here. I'm thinking that data has to be added to the fixture.

In the D6 site, my recollection is that if you use i18n taxonomy to translate terms (as was done in this site that was migrated), the translations are stored with the UI translations, not with the taxonomy terms.

What does "the translations are stored with the UI translations" mean? What table(s) is this referring to?

So for instance if you go in the d6 site to the taxonomy page like admin/content/taxonomy/5, you cannot see the translated taxonomy terms

And this is true on the test fixture as well, for some taxonomies that are not set multilingual. For the ones that are multilingual the translated terms are listed.

However when translations are directly on the term ("localizations" in i18n lingo) that is not yet supported.

If this is true, how does one create translation 'directly on the term'? I have only found one way to translate a taxonomy term.

@jhodgdon, thanks for that. It does help. I thought that is what you meant but I didn't want to make any assumptions.

I think the next step is to get translated terms of the type you have (Is there a name for that type of translation? Localization?) into the source fixture. And to do that I need to know how to do it from the UI. Curiously I too, at least I think I have, have used the i18ntaxonmy module to make the translations but they do not appear in i18n_strings.

Maybe Gábor Hojtsy can explain?

OK, I think I got it. Just uploading a patch for the test fixture, no migrations yet. There is some extra data in it that I think can be removed but I'll look at that later.

A work in progress.

Misspelled the class name.

Fixed coding standard errors, upgrade taxonomy term entity count and change migration to Content (cut/paste error).

Well, more fool me. I've been looking at the errors and not realizing that I wasn't working from HEAD. Starting over.

Remove an error from the fixture, update entity counts.

Wrong namespace on test file.

There are changes to the fixture that need to be removed.

Removed unwanted changes from the test fixture.

@jhodgdon, yes I should have set back to NW as I want to do some further checks.

I've done my checks and it all looks good. To avoid processing duplicate rows this add the same check as in #2225587: Migrate D6 i18n menu links to skip the duplicates in prepareRow.

I want to run another manual test next

Bumping to major as this is a pretty important milestone for i18n migrations. Not sure if it should also be 'Migrate Critical'.

Not sure if it should also be 'Migrate Critical'.

I went back and read the meeting notes and the summary in #2905736: Mark Migrate Drupal as stable According to masipila's summary this should be both 'Migrate Critical' and 'Migrate Drupal'.

All i18n upgrade issues must have the 'i18n-migrate' tag
Issues that are blocking migrate_drupal from being stable for multilingual sites must also be tagged as 'Migrate Critical' and 'Migrate Drupal'

Adding those tags, and let's see if I understood the notes correctly.

Then maybe not, in the next comment catch suggests these i18n issues do not block stability.

Tagging with Migrate critical and Migrate Drupal according to the policy discussion for Migrate Drupal stability criteria.

Quietone, just noticed that I cross-podted with your previous comment.

Taxonomt terms are content. I interpret the policy discussion comments by catch and gabor so that this needs to land before we csn declare Migrate Drupal stable for multilingual sites.

Markus

No longer applies

A reroll

Corrected test failures.

@Jo Fitzgerald, thanks. I just found my error in making the reroll and was about to make a patch!

OK, thanks again Jo Fitzgerald. I think this is ready for manual testing.

@jhodgdon, I hope your offer to test still stands. This is ready for a test.

@jhodgdon, thanks for the quick response. I'm not awake enough to respond to your questions but I have one for you! Was the empty D8 site installed with the standard install or minimal?

The reason for the warning is because we do not audit for conflicts on i18n. Period. It was spun out of #2876085-92: Before upgrading, audit for potential ID conflicts into #2905759: Before upgrading, audit for potential i18n ID conflicts. Since it isn't clear with the current wording, we should update the handbook page to make it more clear about potential issues.

Here are the points I see #50 and then my thoughts.

1) Troublesome Language in the UI.
I agree with jhodgson that this needs improvement. I checked the existing UI issues and there is none that is specific to this problem so a new issue is needed.
2) Source ID 773: Unable to get entity.
Appears unrelated to this issue. Maybe a separate issue to track down what entity is causing the problem?
3) Vocabulary translation not enabled at /admin/config/regional/content-language.
Move this to a follow up. Why? One is that it is a separate migration and will have its own challenges that should not hold this up. Two, these i18n tend to be complicated and best to be done is small steps. Three, I have taken a look and it is not obvious where to the source data will come from for the vocabulary content language settings. There is no equivalent 'translate this vocabulary' in the Drupal 6 dump nor do I see such a thing when reading the i18ntaxonomy module files. Four, this patch hasn't had a review yet so I expect there will be things here to do. Five, I really dislike rerolling patches that have fixture changes and they just get harder the longer it takes to get it committed.
4) Taxonomy terms not shown on node.
Being handled in #2859297: Migrate taxonomy term references for D6 Node translations
5) The translations are present on the edit taxonomy page.
Yay! This is what this issue was to do, migrate the translated taxonomy terms.

Setting to NR for feedback.

A couple of documentation nits.

1. Why are we talking about menu links in the context of taxonomy term migration?

+++ b/core/modules/content_translation/src/Plugin/migrate/source/d6/TermLocalizedTranslation.php
@@ -0,0 +1,99 @@
...
+  public function query() {
+    // Ideally, the query would return rows for each language for each menu link
+    // with the translations for both the title and description or just the
+    // title translation or just the description translation.

2. Same thing here

+  public function fields() {
+    $fields = [
+      'language' => $this->t('Language for this menu.'),

3. Typo

+      'name_translated' => $this->t('Term nametranslation.'),

4. Class documentation should start with a third person verb i.e. 'Tests migration...'

+++ b/core/modules/content_translation/tests/src/Kernel/Migrate/d6/MigrateTermLocalizedTranslationTest.php
@@ -0,0 +1,140 @@
+ * Test migration of translated localized taxonomy terms.

5. Same thing applies to method descriptions

+   * Validate a migrated term contains the expected values.

6. And here

+   * Assert that a term is present in the tree storage, with the right parents.

Cheers,
Markus

@masipla, thanks for the review.

This should fix all points in #57.

Hi! Superb as always @quietone!

@jhodgdon has done manual testing, I have reviewed the patch and my feedback has been addressed. The interdiff shows that the changes were doc changes only since the manual testing so the manual test results are still valid. The test coverage looks good to me.

Of course it doesn't hurt if somebody else can give this another pair of eyeballs but this looks good to me.

Markus

thanks masipila.

One thing about this though is that the migration is in content_translation but the migration does a lookup on d6_taxonomy_vocabulary. If that doesn't exist then migration_lookup returns NULL and the row is skipped. There should probably be a comment to that effect in the migration.

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php
@@ -47247,8 +47453,8 @@
+  'access' => '1521962400',
+  'login' => '1521961881',

This change can be reverted.

+++ b/core/modules/content_translation/src/Plugin/migrate/source/d6/TermLocalizedTranslation.php
@@ -0,0 +1,99 @@
+class TermLocalizedTranslation extends Term {

Based on #60, should we do a module_handler check that taxonomy module is enabled on D8? In this class' checkRequirements?

Since the migration is in content_translation it won't be discovered unless content_translation is enabled. While thinking and poking around I see that d6_node_translation.yml declares 'destination_module: content_translation' which none of the i18n migrations due. That looks like the way to do and then we can put the i18 migrations and source plugins in their respective module directory instead of content_translation. I still just want to check what the UI does in that case, just to be sure.

Assigning to quietone for review.

Yea, that wasn't right. What I didn't realize is that d6_node_translation.yml can be in the node module because the deriver will prevent the creation of the derivatives when content_translation is not enabled. There isn't something similar for i18n migrations, so they go into content_translation and will only be discovered when that is enabled.

should we do a module_handler check that taxonomy module is enabled on D8? In this class' checkRequirements

Surely the source plugin should only have requirements based on the source. Unlike most source plugins the i18n ones require several source tables and there is no check that all those tables exist. Are there any source plugins that check that the source tables exists? Yes, 3, in d6/Node.php, d7/user.php and d6/User.php so it is not a common practice.

So, this patch now puts the files in directories just like the already committed d6_menu_links. The migration goes into content_translation and all other files in the respective module directory. The UI shows the new taxonomy migration only if content_translation is enabled. And there is a bug fix, to get the source plugin to return a count a change was made to the query.

Hmm, I thought I ran the tests. Anyway, correct the namespaces and make sure that the source row has a property 'language'.

Ready for review

@jhodgdon, thanks for the offer. I don't think it is necessary, the last set of changes didn't affect the migration process itself and were done to be consistent with the structure of the existing menu_link translation files and for the UI.

masipila asked me about the changes in #66, specifically about the use of 'ltlanguage' in the query and I see I didn't explain that at all. What I notices was that 'drush ms' was returning a count of 'N/A' for this migration and on closer inspection it wasn't happy about the two language fields in the query, on in locales_target and the other in term data. Adding a field for 'ltlanguage' to the query and then `drush ms` returned to the correct count. And finally, so that a nice property name is available for use, 'ltlanguage' is copied to to the source row as 'language'

I tested this manually to double check that I'm understanding everything correctly.

Drupal 6 setup

1. Installed D6.38
2. Installed https://ftp.drupal.org/files/projects/i18n-6.x-1.10.tar.gz
3. Enabeld Taxonomy translation and the required Internationalization, String translation, Locale, Content translation
4. Added Finnish as a second language with path prefix and language fallback
5. Added a new vocabulary, vocabulary_a with these settings:
- Localize terms. Terms are common for all languages, but their name and description may be localized.
- Added this vocabulary to Story content type
6. Created a new Story node
- Added a new term 'Yellow' and added a description to this term.
7. I can translate this term (title and description) at admin/build/translate
- I translated both Term title and description to Finnish
8. When I view fi/node/1 I can see the Finnish translation for the term i.e. Keltainen

Drupal 8 setup and migrations

1. Applied https://www.drupal.org/files/issues/2018-06-05/2886609-66.patch on D8
- D8 had Finnish added as a second language

2. I executed the following migrations using Drush:
- upgrade_d6_taxonomy_vocabulary
- upgrade_d6_taxonomy_term
- upgrade_d6_taxonomy_term_localized_translation

Migrations executed without errors. The count on drush migrate-status was shown correctly (Re: #70)

3. The vocabulary 'Enable translation' setting was not migrated as pointed out earlier by @jhodgdon but that will be handled as part of #2975509: Migrate D6 vocabulary language settings and not this issue.
- I enabled this setting manually to my vocabulary

4. When I view the taxonomy term translations on D8, I can see that both taxonomy term name and description translations are correctly migrated.

Conclusion

- I have reviewed the patch and all my feedback has been addressed.
- There is test coverage.
- @jhodgdon has tested this manually as well as I have. Both have reported that the manual tests are OK.
- Follow-ups have been created.
- Testbot is green.
- Let's land this. RTBC!

Thank you very much @jhodgdon and @quietone! And thanks @Jo Fitzgerald for your help as well!

Cheers,
Markus

Gábor Hojtsy pointed in Slack that this isn't migrating vocabularies, just the localized taxonomy term therefor updating the title.

Further updates to IS to help committers.

Superb, looks good to me too now. Thanks for making the corrections in the issue summary as well, it is important for everyone reviewing what got committed to understand the exact scope and not complain that vocabularies were not even here for example. Now they have pointers to look at.

Looks like this introduced test failures on all PostgreSQL 9.1 environments:
https://www.drupal.org/pift-ci-job/989536
https://www.drupal.org/pift-ci-job/989684

@tacituseu yep you're right. Reverted. This is likely to need an issue to fix \Drupal\taxonomy\TermStorage::loadTree() to be DB agnostic. ugh.

Fixing typo.

Added clarification that this issue is about the 'localized' concept that we have in D6, not about 'per language' concept.

If I'm not mistaken, we can fix the PostgreSQL error by using explicit type casting, see https://stackoverflow.com/questions/42269987/understanding-explicit-type...

I remeber seeing this approach in other patches but can't remeber exactly where (and I'm writing this with my mobile on my way to work so can't search the codebase easily).

Should we add the explicit type casting to our query or postpone this to the issue @alexpott was thinking out loud in #80? Does that issue already exist?

Note: this issue is tagged as 'Migrate critical' i.e. this migration is needed to declare Migrate Drupal stable.

Markus

Should we add the explicit type casting to our query or postpone this to the issue @alexpott was thinking out loud in #80? Does that issue already exist?

Since it's Migrate critical, I think we should add explicit type casting to work around the Postgres problem, with a comment referencing the follow-up issue (creating it if it doesn't already exist :) That should hopefully be enough to land this.

The issue seems to be (as the test fail error message suggests) that term_data.tid is INT(10) and i18n_strings.objectid is VARCHAR(255). We need to cast tid as VARCHAR and then we should be fine. Assigning this to myself.

Cheers,
Markus

Let's fasten the seatbelts and see what the testbot thinks of this. This works on MariaDB and should work also on PostgreSQL.

Follow-up created as suggested, see #2980294: Make \Drupal\taxonomy\TermStorage::loadTree() to be DB agnostic.

Cheers,
Markus

Patch #86 resolved one of the PostgreSQL errors but others remain. I can't see how the Sqlite failure would be related.

I can't work on this in the coming days so if anyone else can, it's all yours.

Markus

The SQLite failure is unrelated and present since at least January (see https://www.drupal.org/pift-ci-job/866534).

Re #89

Do you mean migrating the vocabulary settings i.e. Localized terms vs. Per language terms? This is handled in #2975509: Migrate D6 vocabulary language settings.

Or do you mean migrating the translation of vocabulary name and description? Was that covered in #2225781: Migrate D6 i18n taxonomy vocabularies? I have not tested this part myself so I don't know if that already works.

Or did you mean something else?

Cheers,
Markus

Needed a reroll.

Change to 8.7.x

Needs work for the Postgres Failure

I like what masiplia did in #86 but I think the cast should be in objectid instead. The errors are showing that the tid is a string where it should be an int. So, lets keep the tid as an int but cast the objectid to a int.

Arg, now I see that I mislabel my screen sessions and was in the wrong environment when running tests locally. Need to start over!

Updated issue summary as part of clarifying the scope for each of the multilingual vocabulary / term migration issues.

This needs another reroll and I started with the patch in #92, deliberately skipping that last patch which just cause more errors.

Lets sort the term parents before the assertion. I don't think the order of the parent ids is critical, if that is true, this should be OK.

Oh, spring has gone to my head and I put those changes in the wrong file. This add some sorting in the tests to make sure the parent ids are in the same order before asserting.

Patch #102 looks promising. Queued for PostgreSQL and SQLite, lets see if the testbot likes it...

Thanks masipila, I thought I had started the PostgreSQL and SQLite tests.

+++ b/core/modules/taxonomy/src/Plugin/migrate/source/d6/TermLocalizedTranslation.php
--- a/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTest.php
+++ b/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTest.php

I must admit I am not sure why the sorting is necessary in this test since this migrations this test runs are not altered by this patch.

Is it because of the new terms being added to the test database? Might be worth a patch with just that change, and with and without the test changes to verify either way.

Setting NW for the idea in #105

This needs a reroll

Let's try that again. I have no idea how I made that mistake.

Those failures are unexpected. What has happened is that #2975509: Migrate D6 vocabulary language settings was committed and that had changes to the same variable, i18ntaxonomy_vocabulary, specifically vocabulary_3_i_2_ , in the fixture which required changes to those two tests. This updates the tests for the new i18ntaxonomy_vocabulary value used in this patch.

It helps to upload the patch too.

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php
@@ -47421,8 +47627,8 @@
+  'access' => '1521962400',
+  'login' => '1521961881',

This change is not necessary, needs to be removed.

Darn. I didn't mean to upload that test only patch. Ignore it.

First, remove the fixture change.

Add PostgreSQL and SQLite tests

Now add a test only patch. This removes everything from the patch in #114 except the test fixture.

Let's look at the failures with the test fixture only patch in #116.

All three, MySQL, PostgreSQL and SQLite have failures in these test files. These failures are the same across all three database types and make sense given the changes to the fixture.

• MigrateUpgrade6Test
• MigrateLanguageContentTaxonomyVocabularySettingsTest
• MigrateVocabularyFieldInstanceTest

Now, PostgreSQL has two extra failures. in MigrateTaxonomyTermTranslationTest and MigrateTaxonomyTermTest. Both of there are sorting type errors as shown in the follow excerpts. So, making changes in the patch in #114 here makes sense.

 MigrateTaxonomyTermTranslationTest
  Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => '4'
-    1 => '5'
+    0 => '5'
+    1 => '4'

 1) Drupal\Tests\taxonomy\Kernel\Migrate\d6\MigrateTaxonomyTermTest::testTaxonomyTerms
Term 6 has correct parents in vocabulary tree
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 4
-    1 => 5
+    0 => '5'
+    1 => '4'

Now, the patch in #114 has an error with PostgreSQL in Drupal\KernelTests\Core\Database\SchemaTest which appears to be unrelated to the changes in #114. Starting a retest of that now.

Good, the retest of the patch in #114 with PostgreSQL passed, it must have been a testbot problem.

So this is all ready for a review. The patch to review is #114

The patch does the necessary migration and to accommodate the different databases the parent id are sorted before asserting them in the tests. This includes sorting in MigrateTaxonomyTermTest, which was unexpected, but a result of the changes to the test fixture , see #118.

Adding #114 as the last patch.

Add postgres and sqlite tests

Assigning for self for review

Patch #66 was RTBCd in #71 and originally committed in #74-75. It was reverted in #78-79 because of PostgreSQL issues (the patch was originally tested only against MySQL).

The PostgreSQL failures were as follows:

1. PostrgreSQL requires columns to be of same type when joining tables

• The issue here was that we were doing a join on term_data.tid = i18n_strings.objectid.
• The type of term_data.tid is INT(10) and the type of i18n_strings.objectid is VARCHAR(255).
• PostgreSQL requires that the types match.
• This was resolved by explicitly type casting term_data.tid.

2. Array sorting errors in the tests when using PostgreSQL

Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => '4'
-    1 => '5'
+    0 => '5'
+    1 => '4'

This was resolved in #102 by sorting the arrays in the tests before the asserts:

+++ b/core/modules/content_translation/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTranslationTest.php
@@ -108,7 +108,12 @@ protected function assertHierarchy($vid, $tid, array $parent_ids) {
 
     $this->assertArrayHasKey($tid, $this->treeData[$vid], "Term $tid exists in taxonomy tree");
     $term = $this->treeData[$vid][$tid];
-    $this->assertEquals($parent_ids, array_filter($term->parents), "Term $tid has correct parents in taxonomy tree");
+    // PostgreSQL, MySQL and SQLite may not return the parent terms in the same
+    // order so sort before testing.
+    sort($parent_ids);
+    $actual_terms = array_filter($term->parents);
+    sort($actual_terms);
+    $this->assertEquals($parent_ids, $actual_terms, "Term $tid has correct parents in taxonomy tree");
   }

And

+++ b/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTest.php
@@ -104,7 +104,14 @@ public function testTaxonomyTerms() {
 
       $this->assertArrayHasKey($tid, $tree_terms, "Term $tid exists in vocabulary tree");
       $tree_term = $tree_terms[$tid];
-      $this->assertEquals($values['parent'], $tree_term->parents, "Term $tid has correct parents in vocabulary tree");
+
+      // PostgreSQL, MySQL and SQLite may not return the parent terms in the
+      // same order so sort before testing.
+      $expected_parents = $values['parent'];
+      sort($expected_parents);
+      $actual_parents = $tree_term->parents;
+      sort($actual_parents);
+      $this->assertEquals($expected_parents, $actual_parents, "Term $tid has correct parents in vocabulary tree");
     }

The sorting topic was investigated further in #103 - #118. The final fix is in #114 and there is a test-only patch in #116. Analysis of this in #118 is accurate in my eyes.

I reviewed patch #114 and it looks good to me. I compared it to the patches #66 which was originally committed.

• The explicit type casting change is in the source plugin, which I manually tested in #86.
• All other changes are in the tests i.e. no need for manual testing.

Tests are green on all three DB backends so this is good to go. RTBC.

Markus

Queued the last patch for a re-test to double check that the patch still applies without re-rolls.

And it didn't apply anymore... Needs re-roll.

Re-rolled.

Thanks for the re-roll Jo!

We currently have tests in core/modules/content_translation/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTranslationTest.php

Patch #114 moves this from content_translation module to taxonomy module:

diff --git a/core/modules/content_translation/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTranslationTest.php b/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTermLocalizedTranslationTest.php
similarity index 63%
copy from core/modules/content_translation/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTranslationTest.php
copy to core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTermLocalizedTranslationTest.php
index 3e32406d28..4c5e216163 100644
--- a/core/modules/content_translation/tests/src/Kernel/Migrate/d6/MigrateTaxonomyTermTranslationTest.php
+++ b/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTermLocalizedTranslationTest.php

In patch #114 we preserve the history of the original tests and only do a diff to the changes.

In the re-rolled patch #128 we're adding everything as a new file:

diff --git a/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTermLocalizedTranslationTest.php b/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTermLocalizedTranslationTest.php
new file mode 100644
index 0000000..4c5e216
--- /dev/null
+++ b/core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateTermLocalizedTranslationTest.php

1. I believe the approach in #114 is how we should do thing.

2. Question: I'm a bit rusty but how did our Migrate Drupal Multilingual things go?
At least in MigrateTermLocalizedTranslationTest we don't enable Migrate Drupal Multilingual at the moment so have we missed something?

+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = [
+    'content_translation',
+    'language',
+    'menu_ui',
+    'node',
+    'taxonomy',
+  ];

Markus

130.1. Personally, I find it isn't always possible to make an interdiff on a reroll. Certainly interdiff itself will fail. Anyway, that is my experience.
130.2 Yes, this should be added. The Multilingual tag is checked in MigrationConfigurationTrait which is for the UI. Originally, that check for the Multilingual tag was in checkRequirements on the source plugin, but that was a bit of a hack and didn't catch the followup migrations so it all got moved to the previously mentioned trait. The tags are now added and test updated.

Looking at the Meta it looks like this is blocking 3 issues
#2746541: Migrate D6 and D7 node revision translations to D8
#2979964: Migrate translations for D7 i18n taxonomy 'localized' terms
#2859297: Migrate taxonomy term references for D6 Node translations
Tagging a blocker

I've read through the patch, IS & comments, I think everything has been address (automated testing, manual testing, postgres failures).
Let's get back at RTBC.

+++ b/core/modules/taxonomy/src/Plugin/migrate/source/d6/TermLocalizedTranslation.php
@@ -0,0 +1,100 @@
+
+    // Add in the property, which is either name or description.
+    // Cast td.tid as char for PostgreSQL compatibility.
+    $query->leftJoin('i18n_strings', 'i18n', 'CAST(td.tid AS CHAR(255)) = i18n.objectid');

This could use an additional explanation as to why it's necessary for postgres compatibility, it's a bit strange having postgres specific code in core, although maybe it's OK to workaround like this for a migration.

We do that in several places in migrations. See MenuLinkTranslation as one example and reference #3001749-23: Migrate D7 i18n custom blocks.

$query->leftJoin(static::I18N_STRING_TABLE, 'i18n', 'CAST(ml.mlid as CHAR(255)) = i18n.objectid');

This code was already committed once and reverted around #77 for postgres failures. This casting just gets us past that.

OK that's fair enough.

Committed and pushed a3072cba96 to 8.7.x and 36ba6ba3de to 8.6.x. Thanks!