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
Comment #1
bjaspan commentedThis 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.
Comment #2
bjaspan commentedForgot to change status.
Comment #3
dries commentedCommitted to CVS HEAD. Thanks.