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
Comment | File | Size | Author |
---|---|---|---|
#12 | 3100046-12.patch | 1.67 KB | alexpott |
#12 | 11-12-interdiff.txt | 791 bytes | alexpott |
#11 | 3100046-11.patch | 1.67 KB | alexpott |
#11 | 3100046-11.test-only.patch | 749 bytes | alexpott |
#6 | 3100046-6.patch | 1.06 KB | Wim Leers |
Comments
Comment #2
super_romeo CreditAttribution: super_romeo commentedComment #3
super_romeo CreditAttribution: super_romeo as a volunteer commentedPatch is not good.
Need more time to think.
Comment #4
super_romeo CreditAttribution: super_romeo as a volunteer commentedComment #5
quietone CreditAttribution: quietone at Acro Commerce commentedComment #6
Wim LeersRan 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.
Comment #7
quietone CreditAttribution: quietone at Acro Commerce commentedA 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.
Comment #8
Wim LeersTotally fair.
Comment #9
quietone CreditAttribution: quietone as a volunteer commentedOh 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
Comment #10
Wim LeersHah! :)
Comment #11
alexpottHere's a test. Perhaps we need to have some generic source plugin count testing...
Comment #12
alexpottCan't spell.
Comment #14
alexpottCommitted 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
Comment #18
quietone CreditAttribution: quietone as a volunteer commented@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?
Comment #19
alexpott@quietone it looks like \Drupal\Tests\field\Kernel\Plugin\migrate\source\d6\FieldOptionTranslationTest never sets the expected count so it's not tested there.
Comment #20
quietone CreditAttribution: quietone as a volunteer commented@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.
Comment #21
alexpottAh 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.