Reported by bangpound in #516138-72: Move CCK HEAD in core:
using field_ui patch,
1. Create a field of any type on bundle X.
2. Add a new instance of that field to bundle Y.
3. Delete the field instance on bundle X.
4. Add the field instance back to bundle X.
--> SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '6-article' for key 2.

Can be reproduced with API calls:

field_create_field(array('field_name' => 'foo', 'type' => 'text'));
field_create_instance(array('field_name' => 'foo', 'bundle' => 'article'));
field_create_instance(array('field_name' => 'foo', 'bundle' => 'page'));
field_delete_instance('foo', 'page);
field_create_instance(array('field_name' => 'foo', 'bundle' => 'page'));

We cannot have a unique key on (field_id, bundle), because of instances marked as deleted but not purged yet.
The only thing we can say is 'there can be only one (field_id, bundle) where deleted = 0', but that's enforced by API, not SQL.

Comments

bjaspan’s picture

StatusFileSize
new1.13 KB

This bug dates back to the original Fields in Core patch. We had the primary key for field_config_instances set to field_name,bundle, which was at least semantically correct at the time. In #392686: Switch to serial primary keys we switched to serial primary keys as a precursor to bulk delete, and the primary key got converted to a unique key; still semantically correct. #364620: Allow creating a field with a deleted field's name allowed creating a field with the same name as a deleted field (a necessary feature and another precursor to bulk delete), and at that time the unique key was no longer correct since (as yched points out) a single field can have rows per instance if some are deleted; also, that patch changed the unique key from using field name to field id, which was wrong because field_delete_instance() in fact uses field_name, not field_id, as the query condition.

Bottom line, the patch is right. I removed a spurious period, otherwise it is RTBC.

bjaspan’s picture

Status: Needs review » Reviewed & tested by the community

Forgot to change status.

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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