Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
So in field_update_field it attempts to determine if the field has data and then passes that into hook_field_update_forbid so that modules can prevent field schema changes if a field has data and schema changes are an issue for them.
field_update_field checks to see if the updated field has data, though not the existing field.
This is a subtle difference, and with the default sql storage engine, means that the EFQ executes just fine, as the DB table has exactly the same name, but if one has a storage engine where say fields are stored in differently named tables for different schema column settings, then the EFQ will attempt to search in the wrong table.
Proposed resolution
Instead of passing do this:
$has_data = field_has_data($field);
We can do this:
$has_data = field_has_data($prior_field);
Remaining tasks
Review the test, and the patch.
User interface changes
None.
API changes
None.
Comments
Comment #1
Steven Jones CreditAttribution: Steven Jones commentedHere's a patch to fix the issue, lets see if anything depends on the broken behavior.
Comment #2
Steven Jones CreditAttribution: Steven Jones commentedTo be clear about what's going on here:
field_has_dataconstructs an EFQ and then runs it to determine if the field has data, but the field data it is passed is for the new field, not the existing field.If the field storage engine cares, and does something like query a different table, then the wrong table will be queried by the EFQ.
Comment #3
Steven Jones CreditAttribution: Steven Jones commentedHere's a patch that just has the test.
Comment #5
Steven Jones CreditAttribution: Steven Jones commentedAnd here's a patch that includes the test and fix.
Comment #6
Steven Jones CreditAttribution: Steven Jones commentedComment #8
Steven Jones CreditAttribution: Steven Jones commentedIncorrect patches above. That'll teach me to do the work in one codebase and then copy/paste to another.
Here's the patch without the fix.
Comment #9
Steven Jones CreditAttribution: Steven Jones commentedAnd then with the fix.
Comment #10
Steven Jones CreditAttribution: Steven Jones commentedComment #11
Steven Jones CreditAttribution: Steven Jones commentedGah, this test clearly isn't working, the one in #8 should have failed.
Comment #12
Steven Jones CreditAttribution: Steven Jones commentedRight, let's try that again eh, with all the bits of code that need to be there to make the test work.
So basically the point of the test is to set it up so that if someone uses the 'new' field settings to do an EFQ, then we get an exception.
Comment #13
Steven Jones CreditAttribution: Steven Jones commented