The error happen by this context:
1. Add field type "text" to bundle "catalog" (entity type "taxonomy_term") name "catalog_info"
2. Create 1 term for "catalog" name "Sony" this seem to be ok
3. Reinstall (disable -> uninstall -> enable) module "taxonomy"
4. Enable module "taxonomy"
5. Create a bundle (vocabulary) name "catalog" (same name with the first one)
6. The bundle "catalog" also has field name "catalog_info"
7. Add 1 term name "Toshiba" for "catalog"
And the error happen:
"PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'taxonomy_term-1-0-0-und' for key 'PRIMARY': INSERT INTO {field_data_field_catalog_info} (entity_type, entity_id, revision_id, bundle, delta, language, field_catalog_info_value, field_catalog_info_format) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7); Array ( [:db_insert_placeholder_0] => taxonomy_term [:db_insert_placeholder_1] => 1 [:db_insert_placeholder_2] => 1 [:db_insert_placeholder_3] => catalog [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => und [:db_insert_placeholder_6] => Toshiba [:db_insert_placeholder_7] => ) in field_sql_storage_field_storage_write() (line 425 of /home/haonguyen/www/testa_og/modules/field/modules/field_sql_storage/field_sql_storage.module)
"
After that, i find one reason for this:
+ Because the the module "taxonomy" doesn't clear the table "field_data_catalog_info" after i reinstall module "taxonomy" so the "duplicate entry error" happen
Comment | File | Size | Author |
---|---|---|---|
#41 | 1115510_uninstall_inc.patch | 7.18 KB | Mile23 |
#37 | 1115510_delete_bundle.patch | 1.1 KB | Mile23 |
#25 | 1115510-uninstall-bundles.patch | 3.97 KB | catch |
#22 | oh_dear.patch | 4.43 KB | catch |
#19 | oh_dear.patch | 4.43 KB | catch |
Comments
Comment #1
catchArggh, thanks for the clear description.
I think this is a missing field_attach_delete_bundle() - that allows the field data to be marked as deleted, so it would be cleared out. This needs to be fixed in 8.x first then backported, but patch will be exactly the same.
Comment #2
howto CreditAttribution: howto commentedThis module is a good solution. Thank "boombatower"
Link: http://drupal.org/project/field_helper
Comment #3
catchThis is still a critical bug IMO.
Comment #4
mrsinguyen CreditAttribution: mrsinguyen commented+1
Comment #5
catchComment module has the same issue.
So - in $entity_providing_module_uninstall() - we need to find out all the bundles - in taxonomy's case this will have to be a direct query against the tables (the same one used in taxonomy_vocabulary_get_names(), in comment module's cases it already calls node_type_get_types(). Then call field_attach_delete_bundle().
This is quite an easy fix, so I'm adding the novice tag to this issue.
Comment #6
barbi CreditAttribution: barbi commentedComment #7
barbi CreditAttribution: barbi commentedNot sure if I have it right, so I ll write my code here, instead of a patch.
Comment #8
cweagansFYI: In order for people to look at a change, you should set it to needs review. :)
Comment #9
barbi CreditAttribution: barbi commentedComment #11
barbi CreditAttribution: barbi commentedQuoting Catch from IRC:
barbi: you should use a direct database query to get the vocabulary names instead of the API function - since taxonomy module won't be enabled at the point that it's disabled.
Comment #12
amateescu CreditAttribution: amateescu commentedLet's fix comment.module too, shall we? :)
Comment #13
catchHere's a patch for the taxonomy module tests, I can't reproduce the exact failure from the OP, and it took a while to get this test to fail, but I get one exception here and I think #12 will fix it.
To clarify exactly what the bug is:
- when you disable then uninstall taxonomy module, it removes all your vocabularies and terms from the database. However it does not inform Field API that the bundles were deleted for taxonomy terms. This leaves instances in the database for bundles that no longer exist.
- If you re-install taxonomy module, there is nothing stopping you creating a vocabulary with the same machine name again.
- If you then add a field instance to terms in that vocabulary, for a field that was previously attached to terms in the old vocabulary that was uninstalled, then things blow up.
The steps to reproduce are very convoluted, but the bug is simple - when you have a module providing an entity type, you need to inform the field API that the bundles are being deleted in hook_uninstall().
Comment #14
catchClearer title to show what's happening.
Comment #16
catchHmm, this may have exposed another issue since field module may not (and probably doesn't) purge instances immediately.
So... either field_modules_uninstalled() should run some field purge code to get that happening, or the test will need to do that for it.
Comment #17
catchfield_purge_batch() doesn't help either in the test or in field_modules_uninstalled(), uploading the in-progress patch in case someone else wants to take over from here. Locally I get the same one exception as above.
Comment #19
catchSo field_attach_delete_bundle() calls field_info_instances(), but field_info_instances() only contains information about bundles that exist in entity_get_info(), and that only includes entities defined by modules that are currently enabled. So when you call field_attach_delete_bundle() from hook_uninstall() it doesn't work. But you wouldn't want to call field_attach_delete_bundle() from hook_disable(), because the bundle isn't being deleted, and there's no concept of disabled bundles.
So now the patch adds the field_attach_delete_bundle() in hook_uninstall(), but also makes field_attach_delete_bundle() work from hook_uninstall(), fun.
At some point we just need to admit that Drupal doesn't and never has properly supported disabling modules, but let's keep patching it up and sending it back out the door again for now..
Comment #20
catchComment #21
aspilicious CreditAttribution: aspilicious commentedtrailing whitespace
Powered by Dreditor.
Comment #22
catchFixed.
Comment #23
yched CreditAttribution: yched commentedDitto about module uninstallation being fundamentally painful with Field cleanup...
And agreed about the fixes.
Is that really needed ? if hook_uninstall calls field_attach_delete_bundle(), then the corresponding instances (and possibly fields) should be marked for deletion and moved out of the way. Unless I'm mistaken, re-enabling the module right away should not cause any issue ? So I don't think we need to attempt this early purge pass (which might not even be enough if there is more than "field_purge_batch_size" values to purge)
Every time field_read_instances() is used over field_info_instances(), we try to add a comment stating the reasons, so that no-one gets tempted to revert that later on (also, to make it clear that field_info_instances() is the "regular" func)
Comment #24
catchThe purge call may not be necessary, that was added before field_read_instances(). Either way
marking cnw to add that comment.
Comment #25
catchTests pass without the purge, removed that and added the comment.
Comment #26
yched CreditAttribution: yched commentedPatch looks good, but actually, re-reading the code, I don't get why field_info_instances() would omit instances on bundles absent from entity_get_info() (i might be missing it, I'm browsing code on a.d.o on a smartphone...)
Comment #27
catchIt uses field_info_bundles(), which uses entity_get_info(), entity_get_info() invokes hook_entity_info(), hooks are only invoked for enabled modules.
?>
Comment #28
yched CreditAttribution: yched commentedIt only uses field_info_bundles() to initialize $info['instances'][$entity_type][$bundle] to an empty array, not to add the instances returned by field_read_instances(), that's done in a separate loop.
Comment #29
catchAh good point.
The bit that does the filtering then is actually in field_read_instances():
_field_info_collate_fields() is not passing 'include_inactive' to field_read_instances(), so it doesn't get the disabled ones (which looks correct to me).
Comment #30
yched CreditAttribution: yched commentedAh, right, forgot about that. That's a notion of 'disabled' that only impacts instances, and that is only detected at runtime (no 'inactive' column in field_config_instances). we might want to move that notion in the db and sync that in cache clear like we do for fields.inactive, but the patch here looks correct for the bugfix.
Dealing with disabled field types, storage types, bundles, entity types,... is such a MASSIVE pain... True in cck, true x4 in d7.
Comment #31
catchDiscussed this in irc with webchick, opened #1187784: Standardize adding a new bundle to an entity type.
Comment #32
webchickYeah, I had concerns about that taxonomy test being a little too specific, and that we would be better served by a more general test that just reinstalled *all* modules exposing fieldable thingies, since this seems like a very simple bug to re-introduce. :\ However, catch pointed out this wasn't as easy as it sounded, because the way these bundles were attached was pretty module-specific.
With that broken into its own issue, I think this one's ready to go. Thanks a lot!!
Committed to 8.x and 7.x.
Comment #34
fagoThat's problematic to implement in the entity API module with pluggable controllers, see #1204820: Entity providing modules must call field_attach_delete_bundle() in hook_uninstall(). I could use some feedback on how to solve it there.. Thanks!
Comment #35
fagouhm, when working on #1204820: Entity providing modules must call field_attach_delete_bundle() in hook_uninstall() I realized this applies to all fieldable entities, even if they don't make use of bundles. This is, as field api creates a default bundle for them. So module developers would have to call field_attach_delete_bundle() for the default bundle in their module_uninstall hook implementation anyway!
That's quite unlogical - I don't have to care about creating, but should care about cleaning up?
Possible fix:
a) Let field API act when a module is uninstalled, manually get entity-info and call the function for modules. That's not reliable though as we would not be able to get reliable entity-info, i.e. alterations would be missing. Maybe, calling field_attach_delete_bundle() independent of the entity-types 'fieldable' flag would be an option?
b) Document that all fieldable entity-providing modules have to call field_attach_delete_bundle().
See related #1330472: clean up entity-related data on module uninstall.
Comment #36
xjmI think the cleanup described in #35 is more properly a task, correct?
Comment #37
Mile23Requiring module developers to call field_attach_delete_bundle() in their hook_uninstall() means that every module that defines a bundle will have the same non-trivial code in its hook_uninstall(). That code will look something like this:
This code could and should live in core, instead.
Comment #38
xjmSetting NR for the bot.
Comment #39
yched CreditAttribution: yched commentedSounds like a good idea, but hardcoding Entity/Field stuff in install.inc is not too cool. Shouldn't this live in some implementation of hook_modules_uninstalled() ?
Comment #40
catchWe can't rely on functions in .module files during uninstall being available, see #1029606: Regression: Not loading the .module file causes a fatal error when uninstalling some modules (as does loading it) for lots of background, so automating won't work here.
Comment #41
Mile23I think we *should* rely on functions in .module files being available during uninstall, because that's the only way this works. :-)
The API says we can define bundles for entities dynamically in modules (see: Node) and that we can alter them programmatically (see: hook_entity_info_alter()). So if we must remove bundles + fields from entities before uninstalling modules, then we need to be able to invoke hook_entity_info() and _alter().
Since the only core function that calls drupal_uninstall_modules() is system_modules_uninstall_submit(), I'd assume that .module file code is available when core uninstalls anything. This leaves external scripts like drush, or modules like Features to uninstall modules, and they'll have to... do something. :-)
My solution: Given that I can only guess as to why uninstall has to live in the same no-module space as install, let's ring in a brand new century and move drupal_uninstall_modules() to a new file called uninstall.inc. An API can grow around it if need be. Also, any module that fails because it doesn't expect to run hook_entity_info() during uninstall should be considered broken.
To my mind, the reasons for putting hook_uninstall() in a module's .install file are a remnant of a bygone time. They're two separate worlds, thanks to field API.
Comment #43
catchMile23. Yes it doesn't work. That's why I have an issue open to remove the ability to disable modules before uninstalling, so we only have to deal with two states instead of three. However a patch that reverts the one linked above is not going to get committed.
Comment #44
tim.plunkettSetting back to CNW.
Comment #45
tim.plunkettI think this is no longer major/relevant due to #1735118: Convert Field API to CMI
Comment #46
barbi CreditAttribution: barbi commentedComment #47
Wtower CreditAttribution: Wtower commentedWhy does it happen that when I call field_attach_delete_bundle() from mymodule_uninstall(), although it does mark field instances a deleted in field_config_instance, they never get deleted? Is this expected?
Comment #65
catchThis was committed a long time ago, and a follow-up could have been created for any follow-up. Marking as fixed again.
I did however find a similar bug again, which is how I rediscovered this one #3409173: Enforced dependency bundles are deleted without validation when modules are uninstalled.