Problem/Motivation

This is a followup for #1735118: Convert Field API to CMI and postponed on that issue.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -0,0 +1,463 @@
+      // Create all per-field-type properties (needed here as long as we have
+      // settings that impact column definitions).
+      $this->settings += $field_type['settings'];

What this is saying is: we are about to create the storage tables based on this new or updated field definition. We need to make sure this particular field has all of the settings keys that are possible for this field type, so that the tables are created with all these columns.

This implies that the columns are created by inspecting the settings array's structure? It seems to me like it would be safer to explicitly use the field type definition when creating the tables, rather than implicitly, and then just set the defaults from the saved field.

@yched replied:

- Absolutely, the schema for the storage tables, defined in hook_field_schema(), depends on $field['settings'], that's totally by design. This merge of default settings is here so that the code in hook_field_schema() doesn't need to do isset($field['settings']['foo']) checks in case the incoming $field definition doesn't specify all the settings. That's nothing new compared to D7 here.

Proposed resolution

Improve the documentation for this snippet and the nearby lines. Review existing field hook API documentation, and reference it where appropriate in Field.

Comments

xjm’s picture

Title: Clarify documentation around field storage and schema in Field::save » Clarify documentation around field storage and schema in Field::save()
Issue tags: +Novice
yched’s picture

Status: Postponed » Fixed

Adressed in the main patch - #1735118-255: Convert Field API to CMI

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