I have a custom module that calls field_update_field(), but that function fails with exceptions like "DatabaseSchemaObjectExistsException: Cannot add index field_sunflower_geo_lat to table field_data_field_sunflower_geo: index already exists." In this case "lat" is the first of many indexes on a geofield.
I'm trying to track down the problem, and I suspect this portion of field_read_fields() may be a problem. Here's a portion of that function:
// Populate storage information.
module_load_install($field['module']);
$schema = (array) module_invoke($field['module'], 'field_schema', $field);
$schema += array(
'columns' => array(),
'indexes' => array(),
);
$field['columns'] = $schema['columns'];
Notice that the code builds $schema['columns'] and $schema['indexes'], then copies only the columns to the field data. Why not also copy the indexes? Later, field_update_fields() looks to $prior_field['indexes'] and finds nothing there!
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | drupal-missing_indexes_field_crud-2003746-2.patch | 1.08 KB | JvE |
Comments
Comment #1
skruf commentedI have the same issue. My problem is that the field collection module adds a new index 'revision_id'. When field_update_field() is called (via features) Drupal doesn't know about this new index as its not added to the $field array so attempts to create it but it already exists.
Comment #2
JvE commentedThis happens when
1. a field is created at a point in time where there are no indexes and the module's hook_field_schema does not contain indexes.
2. at a later point in time hook_field_schema for that module does include indexes.
This situation does occur since field_collection update 7004.
Features that have fields created before that update can no longer be reverted after that update.
To reproduce:
Attached patch should resolve the issue by including indexes from hook_field_schema when doing a field_read_field.
Comment #3
JvE commentedComment #4
dave reidComment #5
scuba_flyI ran into this problem with the relation module ( Relation_endpoint_7002 )
Combined with the #2 patch this fixed my problem.
Since this is core I guess we need an second reviewer?
But +1 RTBC from me.
Comment #6
shrop commentedDescription
A coworker was working on an issue where Field Collection would not run hook_update_7007 due to the error
Cannot add index field_<name>_value to table field_data_field_<name>: index already exists.She said that patch #2 allowed the hook_update to run. I tested and confirmed that the patch resolved the issue in our case since the indexes exist.Thanks for the work on this!
Next Steps
RTBC
Comment #7
shrop commentedComment #8
David_Rothstein commentedIs it possible to write a test for this?
And are we sure there's no similar problem in Drupal 8?
Comment #9
JvE commentedLooks like D8's \Drupal\Core\Field\BaseFieldDefinition::getSchema() and \Drupal\field\Entity\FieldStorageConfig::getSchema() get it right.
And while it may be possible to add a D7 test for this in
\FieldCrudTestCase::testFieldIndexes()(using multiple test modules and/or lots of direct database manipulation) I don't think it's worth the effort.If you want to try:
- Add a new field type, with no indexes in field_test_field_schema().
- Create a field of that type.
- Make field_test_field_schema() somehow return indexes for that field after all.
- Possibly clear some static caches.
- Read the field and see it has no indexes while you expect there to be.
Comment #10
joseph.olstadBumping to 7.61. This didn't make it into 7.60.
Comment #11
joseph.olstadComment #12
joseph.olstadComment #13
mustanggb commentedComment #14
mustanggb commentedComment #15
michfuer commentedComment #16
joseph.olstadComment #17
izmeez commented@joseph.olstad the patch in #2 shows it is passing tests, is there something else that is needed or can it go back to RTBC?
Comment #18
joseph.olstadIf mcdruid sees this perhaps he can decide, he's done a lot of work lately with the db api working on mysql 8 compatibility
Comment #19
mcdruid commentedWill have a closer look at this ASAP, but doesn't look like a specific test was added for this after @David_Rothstein mentioned doing so in #8.