Problem/Motivation

Migration source d7_field_option_translation makes error:

SQLSTATE[42S21]: Column already exists: 1060 Duplicate column name 'type': SELECT COUNT(*) AS expression
FROM
(SELECT DISTINCT fc.*, i18n.*, lt.*, fci.entity_type AS entity_type, fc.type AS type, fci.bundle AS bundle, i18n.lid AS i18n_lid, i18n.type AS i
18n_type, 1 AS expression
FROM
{field_config} fc
INNER JOIN {field_config_instance} fci ON fc.id = fci.field_id
LEFT OUTER JOIN {i18n_string} i18n ON i18n.type = fc.field_name
INNER JOIN {locales_target} lt ON lt.lid = i18n.lid
WHERE (fc.active = :db_condition_placeholder_0) AND (fc.storage_active = :db_condition_placeholder_1) AND (fc.deleted = :db_condition_placeholde
r_2) AND (fci.deleted = :db_condition_placeholder_3) AND (fc.field_name NOT IN (:db_condition_placeholder_4, :db_condition_placeholder_5, :db_co
ndition_placeholder_6, :db_condition_placeholder_7)) AND (i18n.textgroup = :db_condition_placeholder_8) AND (objectid = :db_condition_placeholde
r_9)) subquery; Array
(
[:db_condition_placeholder_0] => 1
[:db_condition_placeholder_1] => 1
[:db_condition_placeholder_2] => 0
[:db_condition_placeholder_3] => 0
[:db_condition_placeholder_4] => title_field
[:db_condition_placeholder_5] => name_field
[:db_condition_placeholder_6] => description_field
[:db_condition_placeholder_7] => subject_field
[:db_condition_placeholder_8] => field
[:db_condition_placeholder_9] => #allowed_values
)

because field_config and i18n_string tables have type columns.

#3100180: Duplicate column name 'type' from d7_field_option_translation source plugin reported the same error message for d7_field_instance_option_translation. Since both of those migrations use the same source plugin, d7_field_option_translation, the fix will apply to both.

Proposed resolution

Write a test for migration d7_field_instance_option_translation.

Remaining tasks

Write a failing test

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

super_romeo created an issue. See original summary.

super_romeo’s picture

super_romeo’s picture

Patch is not good.
Need more time to think.

super_romeo’s picture

Status: Needs review » Active
quietone’s picture

Issue summary: View changes
Wim Leers’s picture

Status: Active » Needs review
Issue tags: +migrate-d7-d8
FileSize
750 bytes
1.06 KB

Ran into this same problem. Pulled my hair out for longer than I'd care to admit figuring out the root cause of this.

The patch does indeed not fix the problem, but it got very close! One detail was forgotten.

quietone’s picture

A while back I looked at this and forgot to comment. I was unable to reproduce the problem, so for me I'd like to see a failing test.

Wim Leers’s picture

Version: 8.8.0 » 8.8.x-dev
Status: Needs review » Needs work
Issue tags: +Needs tests

Totally fair.

quietone’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs tests

Oh my, I was working on #3103031: Add bundle to the sourceIDs to FieldOptionTranslation source plugin and ran into this problem and ended up using that same fix, forgetting that issue exists.

I looked at making a failing test and wasn't able to do it. The tables in the existing source plugin test have the correct columns which should cause the error. It is a bit of a puzzle at the moment.

Still, I have duplicated the problem locally. I applied this patch and it does fix it. Setting

Wim Leers’s picture

Hah! :)

alexpott’s picture

Here's a test. Perhaps we need to have some generic source plugin count testing...

alexpott’s picture

Can't spell.

The last submitted patch, 11: 3100046-11.test-only.patch, failed testing. View results

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 38fdab6bd6 to 9.0.x and 62395e7225 to 8.9.x and 95e728a3c7 to 8.8.x. Thanks!

Backported the bugfix to 8.8.x

  • alexpott committed 38fdab6 on 9.0.x
    Issue #3100046 by alexpott, Wim Leers, super_romeo, quietone: Duplicate...

  • alexpott committed 62395e7 on 8.9.x
    Issue #3100046 by alexpott, Wim Leers, super_romeo, quietone: Duplicate...

  • alexpott committed 95e728a on 8.8.x
    Issue #3100046 by alexpott, Wim Leers, super_romeo, quietone: Duplicate...
quietone’s picture

@alexpott, thanks for that. The source plugin test does test the count. Perhaps the test data in the relevant source plugin test isn't really sufficient?

alexpott’s picture

@quietone it looks like \Drupal\Tests\field\Kernel\Plugin\migrate\source\d6\FieldOptionTranslationTest never sets the expected count so it's not tested there.

quietone’s picture

@alexpott, the source plugin tests do not have to set the expected count for it to be tested. If the expected count is not set in the test or set to NULL the test will set expected_count to the number of rows of expected data. See MigrateSourceTestBase::testSource starting at line 157. A few lines down `$this->assertCount($expected_count, $plugin);` will compare it to the results of the count query.

alexpott’s picture

Ah I see why this is. So it's happening because of \Drupal\Tests\migrate\Kernel\MigrateSqlSourceTestBase::getDatabase() - this creates an SQLite db which allows this query just fine. But the same query breaks on MySQL - core/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldOptionTranslationTest.php doesn't futz with the db driver in the same way as the source plugin tests so it can prove the fix. Fun.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.