Add hook_field_schema_alter()

Reroll for 7.95

This is still working great, including the change to support *.install added in #127.

#129 is the patch to review.

I think the patch looks good in overall.

In addition to the new alter hook addition, there are two changes comparing with the current code:

1.

@@ -388,9 +404,7 @@ function field_read_fields($params = array(), $include_additional = array()) { 
     // Populate storage information.
-    module_load_install($field['module']);
-    $schema = (array) module_invoke($field['module'], 'field_schema', $field);
-    $schema += array('columns' => array(), 'indexes' => array());
+    $schema = field_retrieve_schema($field);

Similar code is on two more places, but here the $schema array does not get 'foreign keys' => array() key inicialized. After the patch is applied, the $schema array gets this key initialized, because the new function field_retrieve_schema() does this by default:

+function field_retrieve_schema($field) {
    ...
+  $schema = (array) module_invoke($field['module'], 'field_schema', $field);
+  $schema += array('columns' => array(), 'indexes' => array(), 'foreign keys' => array());
    ...
+}

I am not suggesting this is wrong or a problem, but better to ensure this specific change would not cause any side-effects.

2.

@@ -130,9 +150,7 @@ function field_create_field($field) {
   // Collect storage information.
-  module_load_install($field['module']);

Before the patch, these functions were loading only one .install file - for the field. After the patch, all .install files are loaded (for obvious reasons), so that any alter hook is found and run:

+function field_retrieve_schema($field) {
+  // Make sure the installation API is available.
+  include_once DRUPAL_ROOT . '/includes/install.inc';
+  module_load_all_includes('install');
    ...
+}

This can have a minor performance effect, but taking into account a fact, that this code is mostly run on field CRUD operations (insert, update, delete) and also in field_read_fields(), where the field definition output is cached in cache_field table most of the time, this should be very minor and not a problem at all.

Just a small nitpick:

diff --git a/modules/field/field.crud.inc b/modules/field/field.crud.inc
@@ -20,6 +20,26 @@
+/**
+ * Retrieves the schema for a field.
+ *
+ * @param array $field
+ *   The field array to get the schema definition against.
+ * @return array
+ *   The field schema definition array.
+ */

I think that the @return part in the phpdoc should be prepended by a newline.

Adding a tag for @Fabianx review.

+1 for patch at #129

Approved!

I am fine with that change.

I agree with the great review, but think that the additional columns should not make a problem in practice.

Thank you everybody that contributed to this.

Very hard to get credit perfect on an issue that's almost old enough to drive a car (in some places) with well over 100 comments; apologies if I've made mistakes.

Hurray, great addition.

I have drafted a simple CR here: https://www.drupal.org/node/3406312 →

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