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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

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.

howto’s picture

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

This module is a good solution. Thank "boombatower"

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

catch’s picture

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

This is still a critical bug IMO.

mrsinguyen’s picture

+1

catch’s picture

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.

barbi’s picture

Assigned: Unassigned » barbi
barbi’s picture

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

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);
  }
}
cweagans’s picture

Status: Active » Needs review

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

barbi’s picture

Status: Needs review » Needs work

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

barbi’s picture

Status: Needs work » Needs review
FileSize
606 bytes

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.

amateescu’s picture

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

catch’s picture

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

catch’s picture

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.

catch’s picture

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.

catch’s picture

Status: Needs work » Needs review
Issue tags: -Novice
FileSize
3.65 KB

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.

catch’s picture

FileSize
4.43 KB

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

catch’s picture

Status: Needs work » Needs review
aspilicious’s picture

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.

catch’s picture

Status: Needs work » Needs review
FileSize
4.43 KB

Fixed.

yched’s picture

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)

catch’s picture

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.

catch’s picture

Status: Needs work » Needs review
FileSize
3.97 KB

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

yched’s picture

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

catch’s picture

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

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();
  }

?>

yched’s picture

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.

catch’s picture

Ah good point.

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


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

yched’s picture

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.

catch’s picture

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

webchick’s picture

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.

fago’s picture

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!

fago’s picture

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.

xjm’s picture

Category: bug » task
Priority: Critical » Major

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

Mile23’s picture

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.

xjm’s picture

Status: Needs work » Needs review

Setting NR for the bot.

yched’s picture

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

catch’s picture

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.

Mile23’s picture

FileSize
7.18 KB

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.

catch’s picture

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.

tim.plunkett’s picture

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

Setting back to CNW.

tim.plunkett’s picture

Priority: Major » Normal

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

barbi’s picture

Wtower’s picture

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?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 7257058 on 8.3.x
    Issue #1115510 by catch, barbi, amateescu: Fixed Entity providing...

  • webchick committed 7257058 on 8.3.x
    Issue #1115510 by catch, barbi, amateescu: Fixed Entity providing...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • webchick committed 7257058 on 8.4.x
    Issue #1115510 by catch, barbi, amateescu: Fixed Entity providing...

  • webchick committed 7257058 on 8.4.x
    Issue #1115510 by catch, barbi, amateescu: Fixed Entity providing...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

  • webchick committed 7257058 on 9.1.x
    Issue #1115510 by catch, barbi, amateescu: Fixed Entity providing...

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

catch’s picture

Status: Needs work » Fixed

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

Status: Fixed » Closed (fixed)

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