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

Files: 
CommentFileSizeAuthor
#41 1115510_uninstall_inc.patch7.18 KBMile23
FAILED: [[SimpleTest]]: [MySQL] 34,468 pass(es), 17 fail(s), and 192 exception(es).
[ View ]
#37 1115510_delete_bundle.patch1.1 KBMile23
PASSED: [[SimpleTest]]: [MySQL] 34,404 pass(es).
[ View ]
#25 1115510-uninstall-bundles.patch3.97 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 30,032 pass(es).
[ View ]
#22 oh_dear.patch4.43 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 30,060 pass(es).
[ View ]
#19 oh_dear.patch4.43 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 30,064 pass(es).
[ View ]
#17 1115510_field_purge_batch.patch3.65 KBcatch
FAILED: [[SimpleTest]]: [MySQL] 30,055 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
#13 tests_only.patch1.8 KBcatch
FAILED: [[SimpleTest]]: [MySQL] 30,050 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
#13 1115510-tests_and_fix.patch2.98 KBcatch
FAILED: [[SimpleTest]]: [MySQL] 30,040 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
#12 1115510-remove_taxonomy_and_comment_bundles-12.patch1.18 KBamateescu
PASSED: [[SimpleTest]]: [MySQL] 29,993 pass(es).
[ View ]
#11 reinstall_module_taxonomy-1115510.patch606 bytesbarbi
PASSED: [[SimpleTest]]: [MySQL] 29,779 pass(es).
[ View ]
#9 reinstall_module_taxonomy-1115510.patch577 bytesbarbi
FAILED: [[SimpleTest]]: [MySQL] 29,760 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Comments

Version:7.0» 8.x-dev
Issue tags:+needs backport to D7

Arggh, 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.

Version:8.x-dev» 7.0
Status:Active» Fixed

This module is a good solution. Thank "boombatower"

Link: http://drupal.org/project/field_helper

Version:7.0» 8.x-dev
Status:Fixed» Active

This is still a critical bug IMO.

+1

Issue tags:+Novice

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

Assigned:Unassigned» barbi

Not sure if I have it right, so I ll write my code here, instead of a patch.

<?php
function taxonomy_uninstall() {
 
// Remove variables.
 
variable_del('taxonomy_override_selector');
 
variable_del('taxonomy_terms_per_page_admin');
 
//Remove taxonomy vocabularies
 
$vocabularies = taxonomy_get_vocabularies();
  foreach (
$vocabularies as $vocabulary) {
   
field_attach_delete_bundle('taxonomy_term', $vocabulary->machine_name);
  }
}
?>

Status:Active» Needs review

FYI: In order for people to look at a change, you should set it to needs review. :)

StatusFileSize
new577 bytes
FAILED: [[SimpleTest]]: [MySQL] 29,760 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Status:Needs review» Needs work

The last submitted patch, reinstall_module_taxonomy-1115510.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new606 bytes
PASSED: [[SimpleTest]]: [MySQL] 29,779 pass(es).
[ View ]

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

Issue tags:-duplicate entry
StatusFileSize
new1.18 KB
PASSED: [[SimpleTest]]: [MySQL] 29,993 pass(es).
[ View ]

Let's fix comment.module too, shall we? :)

StatusFileSize
new2.98 KB
FAILED: [[SimpleTest]]: [MySQL] 30,040 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
new1.8 KB
FAILED: [[SimpleTest]]: [MySQL] 30,050 pass(es), 0 fail(s), and 1 exception(es).
[ View ]

Here'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().

Title: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry..." when reinstall module taxonomy with textfield "Entity providing modules must call field_attach_delete_bundle() in hook_uninstall()

Clearer title to show what's happening.

Status:Needs review» Needs work

The last submitted patch, 1115510-tests_and_fix.patch, failed testing.

Hmm, 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.

Status:Needs work» Needs review
Issue tags:-Novice
StatusFileSize
new3.65 KB
FAILED: [[SimpleTest]]: [MySQL] 30,055 pass(es), 0 fail(s), and 1 exception(es).
[ View ]

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

Status:Needs review» Needs work

The last submitted patch, 1115510_field_purge_batch.patch, failed testing.

StatusFileSize
new4.43 KB
PASSED: [[SimpleTest]]: [MySQL] 30,064 pass(es).
[ View ]

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

Status:Needs work» Needs review

Status:Needs review» Needs work

+++ b/modules/field/field.moduleundefined
@@ -379,6 +379,11 @@ function field_cron() {
+  // Uninstalled modules may delete bundles with active instances. Try to purge ¶

trailing whitespace

Powered by Dreditor.

Status:Needs work» Needs review
StatusFileSize
new4.43 KB
PASSED: [[SimpleTest]]: [MySQL] 30,060 pass(es).
[ View ]

Fixed.

Ditto about module uninstallation being fundamentally painful with Field cleanup...
And agreed about the fixes.

function field_modules_uninstalled($modules) {
   module_load_include('inc', 'field', 'field.crud');
+  // Uninstalled modules may delete bundles with active instances. Try to purge
+  // those instances now in case the module is re-installed again.
+  $limit = variable_get('field_purge_batch_size', 10);
+  field_purge_batch($limit);
+

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)

function field_attach_delete_bundle($entity_type, $bundle) {
   // First, delete the instances themseves.
-  $instances = field_info_instances($entity_type, $bundle);
+  $instances = field_read_instances(array('entity_type' => $entity_type, 'bundle' => $bundle), array('include_inactive' => 1));

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)

Status:Needs review» Needs work

The purge call may not be necessary, that was added before field_read_instances(). Either way
marking cnw to add that comment.

Status:Needs work» Needs review
StatusFileSize
new3.97 KB
PASSED: [[SimpleTest]]: [MySQL] 30,032 pass(es).
[ View ]

Tests pass without the purge, removed that and added the comment.

Patch 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...)

It uses field_info_bundles(), which uses entity_get_info(), entity_get_info() invokes hook_entity_info(), hooks are only invoked for enabled modules.

<?php
function _field_info_collate_fields() {
...
     
// Populate 'instances'. Only non-deleted instances are considered.
     
$info['instances'] = array();
      foreach (
field_info_bundles() as $entity_type => $bundles) {
        foreach (
$bundles as $bundle => $bundle_info) {
         
$info['instances'][$entity_type][$bundle] = array();
        }
      }
..
}
function
field_info_bundles($entity_type = NULL) {
 
$info = entity_get_info();
  if (
$entity_type) {
    return isset(
$info[$entity_type]['bundles']) ? $info[$entity_type]['bundles'] : array();
  }
?>

?>

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

Ah good point.

The bit that does the filtering then is actually in field_read_instances():

<?php
foreach ($results as $record) {
   
// Filter out instances on unknown entity types (for instance because the
    // module exposing them was disabled).
   
$entity_info = entity_get_info($record['entity_type']);
    if (
$include_inactive || $entity_info) {
?>

_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).

Status:Needs review» Reviewed & tested by the community

Ah, 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.

Discussed this in irc with webchick, opened #1187784: Standardize adding a new bundle to an entity type.

Status:Reviewed & tested by the community» Fixed

Yeah, 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.

Status:Fixed» Closed (fixed)

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

That'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!

Status:Closed (fixed)» Needs work

uhm, 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.

Category:bug» task
Priority:Critical» Major

I think the cleanup described in #35 is more properly a task, correct?

StatusFileSize
new1.1 KB
PASSED: [[SimpleTest]]: [MySQL] 34,404 pass(es).
[ View ]

Requiring 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:

  if (module_exists('entity')) {
    $entity_modules = array_intersect($module_list, module_implements('entity_info'));
    foreach($entity_modules as $module) {
      // Invoke hook_entity_info() for all our modules.
      $entities = module_invoke($module, 'entity_info');
      // Invoke hook_entity_info_alter().
      drupal_alter('entity_info', $entities);
      foreach($entities as $entity) {
        foreach($entity['bundles'] as $bundle) {
          field_attach_delete_bundle(key($entity), key($bundle));
        }
      }
    }
  }

This code could and should live in core, instead.

Status:Needs work» Needs review

Setting NR for the bot.

Sounds 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() ?

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

StatusFileSize
new7.18 KB
FAILED: [[SimpleTest]]: [MySQL] 34,468 pass(es), 17 fail(s), and 192 exception(es).
[ View ]

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

Status:Needs review» Needs work

The last submitted patch, 1115510_uninstall_inc.patch, failed testing.

Status:Needs work» Needs review

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

Assigned:barbi» Unassigned
Status:Needs review» Needs work

Setting back to CNW.

Priority:Major» Normal

I think this is no longer major/relevant due to #1735118: Convert Field API to CMI

Why 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?