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.
When a user is denied access to a field that stores a numeric value (e.g., Interger, Taxonomy, etc), on submit of the form (which does not show the denied field) Field API attempts to write an empty string to the DB, which causes a PDO error like this:
PDOException: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' FOR COLUMN 'field_taxonomy_categories_value' at row 1: INSERT INTO {field_data_field_taxonomy_categories} (etid, entity_id, revision_id, bundle, delta, LANGUAGE, field_taxonomy_categories_value) 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); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => 7 [:db_insert_placeholder_2] => 7 [:db_insert_placeholder_3] => q_a [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => zxx [:db_insert_placeholder_6] => ) IN field_sql_storage_field_storage_write() (line 421 of /var/www/vc/drupal-head/modules/FIELD/modules/field_sql_storage/field_sql_storage.module).
I've tried to trace this, and I know that the appropriate (module)_field_is_empty function is being called, and field_default_submit is unsetting the $item, but this is not being relayed to the ultimate database actions.
I'd love to get some guidance from a more experienced Core dev so that I can write a patch for this, but I'm stuck. Any help?
Comment | File | Size | Author |
---|---|---|---|
#13 | field_ui_default_values-619230-13.patch | 15.01 KB | yched |
#8 | field_ui_default_values-619230-8.patch | 15 KB | yched |
#7 | 619230_field_set_default_empty.01.patch | 1 KB | matt2000 |
#5 | 619230_field_set_default_empty.patch | 837 bytes | matt2000 |
Comments
Comment #1
matt2000 CreditAttribution: matt2000 commentedComment #2
yched CreditAttribution: yched commentedWas not able to reproduce at first with an API-created integer field.
But then I thought 'wait a sec', and tried with a field created through the UI. Boom on node create form.
That's because the 'default value' input in 'field settings' form is submitted blank, and field_ui stores '' as the default value.
Then, on 'node add' form submit, field_default_insert() sees there's a default value and tries to insert it.
Pondering on the right fix for this.
Comment #3
matt2000 CreditAttribution: matt2000 commentedDo we need to call [module]_field_is_empty on the default value field and use that to turn '' into NULL where appropriate?
Comment #4
yched CreditAttribution: yched commentedRight, running field_set_empty() on $instance_values['default_value'] in field_ui_field_edit_form_submit() should do the trick.
Ideally, we should run field_default_submit() (with $obj_type and $object = NULL). That would take care of the field_set_empty() call, and more generally ensure that we go through the same processing than during a regular '[node|comment|user|...] form' submit.
Comment #5
matt2000 CreditAttribution: matt2000 commentedYup, that seems to do the trick. Here's the patch.
Comment #6
yched CreditAttribution: yched commentedCan we try with field_default_submit() ? I think that would be the real clean fix.
Comment #7
matt2000 CreditAttribution: matt2000 commentedIs this the right idea? It seems to work, but I'm not sure I understand exactly what happens with the normal hook_field_* stuff, so this feels out of place. (Nothing else calls field_default_submit directly.)
Comment #8
yched CreditAttribution: yched commentedIt's the right direction indeed.
The bug was actually larger. The validation and submission of the 'default value' widget had not been updated for from CCK to D7's field validation workflow.
Attached patch fixes this, and adds tests.
Comment #9
matt2000 CreditAttribution: matt2000 commentedOK, I can't say that I really understand this stuff, so all I found was nit picks. However, I can confirm that the patch fixes the original issue.
Looks like a non-change here.
whitespace changes?
What's this have to do with default values? I don't get it.
I'm on crack. Are you, too?
Comment #10
yched CreditAttribution: yched commented1) Yes, just a result of reshuffling stuff back and forth in the function, ended up in a no-op like that. That's life ;-)
2) and 3). Cleaning up while I'm in there. I try to avoid polluting patches with unrelated cleanups in actual module files, but test files get less attention, and I don't see myself rolling such minor stuff in a separate patch. So if that's OK, I'd rather keep those in.
Comment #11
yched CreditAttribution: yched commentedBump - would be cool to have this in to move forward on other cleanups ;-)
Comment #12
sunLooks good. Only some very minor remarks:
This should be
Form validation handler for field instance settings form.
This should be
Form submit handler for field instance settings form.
(minor) Duplicate spaces here.
uhm - why do we concatenate an empty string?
In general, we use "Verify" instead of "Check" in such inline comments.
I'm on crack. Are you, too?
Comment #13
yched CreditAttribution: yched commentedFixed all points except for
// Check
224 occurrences of
// Check ...
in core .test files (that's without field and field_ui that use it all over the place), vs. 173 occurrences of// Verify ...
;-)Comment #14
sunThanks! :)
Comment #15
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!