- Patch #415044 by bjaspan, yched: indexes for field storage.

9 files changed, 222 insertions, 52 deletions

diff --git a/modules/field/field.api.php b/modules/field/field.api.php
index 130562858..b73e4707f 100644
--- a/modules/field/field.api.php
+++ b/modules/field/field.api.php
@@ -120,15 +120,25 @@ function hook_field_info() {
  * @param $field
  *   A field structure.
  * @return
- *   A Field API schema is an array of Schema API column
- *   specifications, keyed by field-independent column name. For
- *   example, a field may declare a column named 'value'. The SQL
- *   storage engine may create a table with a column named
- *   <fieldname>_value_0, but the Field API schema column name is
- *   still 'value'.
- */
-function hook_field_columns($field) {
-  if ($field['type'] == 'textarea') {
+ *   An associative array with the following keys:
+ *   - 'columns': an array of Schema API column specifications, keyed by
+ *     column name. This specifies what comprises a value for a given field.
+ *     For example, a value for a number field is simply 'value', while a
+ *     value for a formatted text field is the combination of 'value' and
+ *     'format'.
+ *     It is recommended to avoid having the columns definitions depend on
+ *     field settings when possible.
+ *     No assumptions should be made on how storage engines internally use the
+ *     original column name to structure their storage.
+ *   - 'indexes': an array of Schema API indexes definitions. Only columns that
+ *     appear in the 'columns' array are allowed.
+ *     Those indexes will be used as default indexes. Callers of
+ *     field_create_field() can specify additional indexes, or, at their own
+ *     risk, modify the default indexes specified by the field-type module.
+ *     Some storage engines might not support indexes.
+ */
+function hook_field_schema($field) {
+  if ($field['type'] == 'text_long') {
     $columns = array(
       'value' => array(
         'type' => 'text',
@@ -153,7 +163,12 @@ function hook_field_columns($field) {
       'not null' => FALSE,
     ),
   );
-  return $columns;
+  return array(
+    'columns' => $columns,
+    'indexes' => array(
+      'format' => array('format'),
+    ),
+  );
 }
 
 /**
diff --git a/modules/field/field.crud.inc b/modules/field/field.crud.inc
index 9daeebd51..5e7d20c69 100644
--- a/modules/field/field.crud.inc
+++ b/modules/field/field.crud.inc
@@ -65,6 +65,12 @@
  *     exactly like Schema API column specifications but, depending on
  *     the field storage module in use, the name of the column may not
  *     represent an actual column in an SQL database.
+ * - indexes (array).
+ *     An array of indexes on data columns, using the same definition format
+ *     as Schema API index specifications. Only columns that appear in the
+ *     'columns' setting are allowed. Note that field types can specify
+ *     default indexes, which can be modified or added to when
+ *     creating a field. 
  * - settings (array)
  *     A sub-array of key/value pairs of field-type-specific settings. Each
  *     field type module defines and documents its own field settings.
@@ -172,12 +178,25 @@
  */
 
 /**
- * Create a field. This function does not bind the field to any
- * bundle; use field_create_instance for that.
+ * Create a field.
+ *
+ * This function does not bind the field to any bundle; use
+ * field_create_instance() for that.
  *
  * @param $field
- *   A field structure. The field_name and type properties are
- *   required.
+ *   A field structure. The field_name and type properties are required.
+ *   Other properties, if omitted, will be given the following default values:
+ *   - cardinality: 1
+ *   - locked: FALSE
+ *   - indexes: the field-type indexes, specified by the field type's
+ *     hook_field_schema(). The indexes specified in $field are added
+ *     to those default indexes. It is possible to override the
+ *     definition of a field-type index by providing an index with the
+ *     same name, or to remove it by redefining it as an empty array
+ *     of columns. Overriding field-type indexes should be done
+ *     carefully, for it might seriously affect the site's performance. 
+ *   - settings: each omitted setting is given the default value defined in
+ *     hook_field_info().
  * @throw
  *   FieldException
  */
@@ -215,18 +234,37 @@ function field_create_field($field) {
     'locked' => FALSE,
     'settings' => array(),
   );
-  $module = $field_type['module'];
   // Create all per-field-type properties (needed here as long as we have
   // settings that impact column definitions).
   $field['settings'] += field_info_field_settings($field['type']);
-  $field['module'] = $module;
+  $field['module'] = $field_type['module'];
   $field['active'] = 1;
   $field['deleted'] = 0;
-  // Create the data table. We need to populate the field columns, even though
-  // we don't actually store them.
-  $field['columns'] = (array) module_invoke($field['module'], 'field_columns', $field);
+
+  // Collect storage information.
+  $schema = (array) module_invoke($field['module'], 'field_schema', $field);
+  $schema += array('columns' => array(), 'indexes' => array());
+
+  // 'columns' are hardcoded in the field type.
+  $field['columns'] = $schema['columns'];
+
+  // 'indexes' can be both hardcoded in the field type, and specified in the
+  // incoming $field definition.
+  $field += array(
+    'indexes' => array(),
+  );
+  $field['indexes'] += $schema['indexes'];
+
+  // Inform the storage engine.
   module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_create_field', $field);
 
+  // The serialized 'data' column contains everything from $field that does not
+  // have its own column and is not automatically populated when the field is
+  // read.
+  $data = $field;
+  unset($data['columns'], $data['field_name'], $data['type'], $data['locked'], $data['module'], $data['active'], $data['deleted']);
+  $field['data'] = $data;
+
   drupal_write_record('field_config', $field);
 
   // Clear caches
@@ -285,13 +323,17 @@ function field_read_fields($params = array(), $include_additional = array()) {
   $fields = array();
   $results = $query->execute();
   foreach ($results as $field) {
-    // drupal_write_record() writes an empty string for empty arrays.
-    $field['settings'] = $field['settings'] ? unserialize($field['settings']) : array();
+    // Extract serialized data.
+    $data = unserialize($field['data']);
+    unset($field['data']);
+    $field += $data;
 
     module_invoke_all('field_read_field', $field);
 
-    // Populate storage columns.
-    $field['columns'] = (array) module_invoke($field['module'], 'field_columns', $field);
+    // Populate storage information.
+    $schema = (array) module_invoke($field['module'], 'field_schema', $field);
+    $schema += array('columns' => array(), 'indexes' => array());
+    $field['columns'] = $schema['columns'];
 
     $fields[$field['field_name']] = $field;
   }
@@ -326,8 +368,29 @@ function field_delete_field($field_name) {
  * Creates an instance of a field, binding it to a bundle.
  *
  * @param $instance
- *   A field instance structure. The field_name and bundle properties
- *   are required.
+ *   A field instance structure. The field_name and bundle properties are
+ *   required.
+ *   Other properties, if omitted, will be given the following default values:
+ *   - label: the field name
+ *   - description: empty string
+ *   - weight: 0
+ *   - required: FALSE
+ *   - default_value_function: empty string
+ *   - settings: each omitted setting is given the default value specified in
+ *     hook_field_info().
+ *   - widget:
+ *     - type: the default widget specified in hook_field_info().
+ *     - settings: each omitted setting is given the default value specified in
+ *       hook_field_widget_info().
+ *   - display:
+ *     Settings for the 'full' build mode will be added, and each build mode
+ *     will be completed with the follwong default values:
+ *     - label: 'above'
+ *     - exclude: FALSE
+ *       TODO This is subject to change, see: http://drupal.org/node/367215
+ *     - type: the default formatter specified in hook_field_info().
+ *     - settings: each omitted setting is given the default value specified in
+ *       hook_field_formatter_info().
  * @throw
  *   FieldException
  */
@@ -465,8 +528,9 @@ function _field_write_instance($instance, $update = FALSE) {
     $instance['display'][$context]['settings'] += field_info_formatter_settings($instance['display'][$context]['type']);
   }
 
-  // Create $data to contain everything from $instance that does not
-  // have its own column, and thus will be stored serialized.
+  // The serialized 'data' column contains everything from $instance that does
+  // not have its own column and is not automatically populated when the
+  // instance is read.
   $data = $instance;
   unset($data['id'], $data['field_id'], $data['field_name'], $data['bundle'], $data['widget']['type'], $data['weight'], $data['deleted']);
 
@@ -486,7 +550,8 @@ function _field_write_instance($instance, $update = FALSE) {
   if ($update) {
     $record['id'] = $instance['id'];
     $primary_key = array('id');
-  } else {
+  }
+  else {
     $primary_key = array();
   }
   drupal_write_record('field_config_instance', $record, $primary_key);
diff --git a/modules/field/field.install b/modules/field/field.install
index 059376b43..5ac23bc96 100644
--- a/modules/field/field.install
+++ b/modules/field/field.install
@@ -44,7 +44,7 @@ function field_schema() {
         'default' => 0,
         'description' => '@TODO',
       ),
-      'settings' => array(
+      'data' => array(
         'type' => 'text',
         'size' => 'medium',
         'not null' => TRUE,
diff --git a/modules/field/field.test b/modules/field/field.test
index 3f8b9da19..a473d0c77 100644
--- a/modules/field/field.test
+++ b/modules/field/field.test
@@ -1096,7 +1096,7 @@ class FieldTestCase extends DrupalWebTestCase {
     // Check that field type is required.
     try {
       $field_definition = array(
-        'field_name' => drupal_strtolower($this->randomName()),
+        'field_name' => 'field_1',
       );
       field_create_field($field_definition);
       $this->fail(t('Cannot create a field with no type.'));
@@ -1116,7 +1116,7 @@ class FieldTestCase extends DrupalWebTestCase {
     }
 
     $field_definition = array(
-      'field_name' => drupal_strtolower($this->randomName()),
+      'field_name' => 'field_2',
       'type' => 'test_field',
     );
     field_create_field($field_definition);
@@ -1145,7 +1145,10 @@ class FieldTestCase extends DrupalWebTestCase {
     }
 
     // Check that invalid field names are rejected.
-    $field_definition['field_name'] .= '_#';
+    $field_definition = array(
+      'field_name' => 'field_#',
+      'type' => 'test_field',
+    );
     try {
       field_create_field($field_definition);
       $this->fail(t('Cannot create a field with an invalid name.'));
@@ -1153,10 +1156,52 @@ class FieldTestCase extends DrupalWebTestCase {
     catch (FieldException $e) {
       $this->pass(t('Cannot create a field with an invalid name.'));
     }
-
     // TODO : other failures
   }
 
+  /**
+   * Test creation of indexes on data column.
+   */
+  function testFieldIndexes() {
+    // Check that indexes specified by the field type are used by default.
+    $field_definition = array(
+      'field_name' => 'field_1',
+      'type' => 'test_field',
+    );
+    field_create_field($field_definition);
+    $field = field_read_field($field_definition['field_name']);
+    $expected_indexes = array('value' => array('value'));
+    $this->assertEqual($field['indexes'], $expected_indexes, t('Field type indexes saved by default'));
+
+    // Check that indexes specified by the field definition override the field
+    // type indexes.
+    $field_definition = array(
+      'field_name' => 'field_2',
+      'type' => 'test_field',
+      'indexes' => array(
+        'value' => array(),
+      ),
+    );
+    field_create_field($field_definition);
+    $field = field_read_field($field_definition['field_name']);
+    $expected_indexes = array('value' => array());
+    $this->assertEqual($field['indexes'], $expected_indexes, t('Field definition indexes override field type indexes'));
+
+    // Check that indexes specified by the field definition add to the field
+    // type indexes.
+    $field_definition = array(
+      'field_name' => 'field_3',
+      'type' => 'test_field',
+      'indexes' => array(
+        'value_2' => array('value'),
+      ),
+    );
+    field_create_field($field_definition);
+    $field = field_read_field($field_definition['field_name']);
+    $expected_indexes = array('value' => array('value'), 'value_2' => array('value'));
+    $this->assertEqual($field['indexes'], $expected_indexes, t('Field definition indexes are merged with field type indexes'));
+  }
+
   function testReadField() {
 
   }
@@ -1168,9 +1213,9 @@ class FieldTestCase extends DrupalWebTestCase {
     // TODO: Also test deletion of the data stored in the field ?
 
     // Create two fields (so we can test that only one is deleted).
-    $this->field = array('field_name' => 'test_field_name', 'type' => 'test_field');
+    $this->field = array('field_name' => 'field_1', 'type' => 'test_field');
     field_create_field($this->field);
-    $this->another_field = array('field_name' => 'another_test_field_name', 'type' => 'test_field');
+    $this->another_field = array('field_name' => 'field_2', 'type' => 'test_field');
     field_create_field($this->another_field);
 
     // Create instances for each.
diff --git a/modules/field/modules/field_sql_storage/field_sql_storage.module b/modules/field/modules/field_sql_storage/field_sql_storage.module
index 7a774f028..51a82308a 100644
--- a/modules/field/modules/field_sql_storage/field_sql_storage.module
+++ b/modules/field/modules/field_sql_storage/field_sql_storage.module
@@ -57,6 +57,21 @@ function _field_sql_storage_columnname($name, $column) {
 }
 
 /**
+ * Generate an index name for a field data table.
+ *
+ * @param $name
+ *   The name of the field
+ * @param $column
+ *   The name of the index
+ * @return
+ *   A string containing a generated index name for a field data
+ *   table that is unique among all other fields.
+ */
+function _field_sql_storage_indexname($name, $index) {
+  return $name . '_' . $index;
+}
+
+/**
  * Retrieve or assign an entity type id for an object type.
  *
  * @param $obj_type
@@ -78,8 +93,8 @@ function _field_sql_storage_etid($obj_type) {
 /**
  * Return the database schema for a field. This may contain one or
  * more tables. Each table will contain the columns relevant for the
- * specified field. Leave $field['columns'] empty to get only the
- * base schema.
+ * specified field. Leave the $field's 'columns' and 'indexes' keys
+ * empty to get only the base schema.
  *
  * @param $field
  *   The field structure for which to generate a database schema.
@@ -134,8 +149,17 @@ function _field_sql_storage_schema($field) {
   );
 
   // Add field columns.
-  foreach ($field['columns'] as $column_name => $attributes) {
-    $current['fields'][_field_sql_storage_columnname($field['field_name'], $column_name)] = $attributes;
+  foreach ((array) $field['columns'] as $column_name => $attributes) {
+    $real_name = _field_sql_storage_columnname($field['field_name'], $column_name);
+    $current['fields'][$real_name] = $attributes;
+  }
+
+  // Add indexes.
+  foreach ((array) $field['indexes'] as $index_name => $columns) {
+    $real_name = _field_sql_storage_indexname($field['field_name'], $index_name);
+    foreach ($columns as $column_name) {
+      $current['indexes'][$real_name][] = _field_sql_storage_columnname($field['field_name'], $column_name);
+    }
   }
 
   // Construct the revision table. The primary key includes
diff --git a/modules/field/modules/list/list.module b/modules/field/modules/list/list.module
index ba793476d..615165f41 100644
--- a/modules/field/modules/list/list.module
+++ b/modules/field/modules/list/list.module
@@ -59,7 +59,7 @@ function list_field_info() {
 /**
  * Implementation of hook_field_schema().
  */
-function list_field_columns($field) {
+function list_field_schema($field) {
   switch ($field['type']) {
     case 'list_text':
       $columns = array(
@@ -89,7 +89,12 @@ function list_field_columns($field) {
       );
       break;
   }
-  return $columns;
+  return array(
+    'columns' => $columns,
+    'indexes' => array(
+      'value' => array('value'),
+    ),
+  );
 }
 
 /**
diff --git a/modules/field/modules/number/number.module b/modules/field/modules/number/number.module
index 799853c5b..34bfd9eae 100644
--- a/modules/field/modules/number/number.module
+++ b/modules/field/modules/number/number.module
@@ -48,7 +48,10 @@ function number_field_info() {
   );
 }
 
-function number_field_columns($field) {
+/**
+ * Implementation of hook_field_schema().
+ */
+function number_field_schema($field) {
   switch ($field['type']) {
     case 'number_integer' :
       $columns = array(
@@ -79,7 +82,9 @@ function number_field_columns($field) {
       );
       break;
   }
-  return $columns;
+  return array(
+    'columns' => $columns,
+  );
 }
 
 /**
diff --git a/modules/field/modules/text/text.module b/modules/field/modules/text/text.module
index 7b67b11c4..a5cd9f35b 100644
--- a/modules/field/modules/text/text.module
+++ b/modules/field/modules/text/text.module
@@ -57,7 +57,7 @@ function text_field_info() {
 /**
  * Implementation of hook_field_schema().
  */
-function text_field_columns($field) {
+function text_field_schema($field) {
   if ($field['type'] == 'text_long') {
     $columns = array(
       'value' => array(
@@ -83,7 +83,12 @@ function text_field_columns($field) {
       'not null' => FALSE,
     ),
   );
-  return $columns;
+  return array(
+    'columns' => $columns,
+    'indexes' => array(
+      'format' => array('format'),
+    ),
+  );
 }
 
 /**
diff --git a/modules/simpletest/tests/field_test.module b/modules/simpletest/tests/field_test.module
index 961b62f4c..7a3b544c6 100644
--- a/modules/simpletest/tests/field_test.module
+++ b/modules/simpletest/tests/field_test.module
@@ -348,15 +348,21 @@ function field_test_field_info() {
 }
 
 /**
- * Implementation of hook_field_columns().
+ * Implementation of hook_field_schema().
  */
-function field_test_field_columns($field) {
-  $columns['value'] = array(
-    'type' => 'int',
-    'size' => 'tiny',
-    'not null' => FALSE,
+function field_test_field_schema($field) {
+  return array(
+    'columns' => array(
+      'value' => array(
+        'type' => 'int',
+        'size' => 'tiny',
+        'not null' => FALSE,
+      ),
+    ),
+    'indexes' => array(
+      'value' => array('value'),
+    ),
   );
-  return $columns;
 }
 
 /**