Rename field language properties to langcode

You can use

drush php-eval "print_r(drupal_get_complete_schema());"

to check for language.

http://api.drupal.org/api/drupal/core!includes!bootstrap.inc/function/dr...

Using patch drush: #1396178: Add properties output as a --format and

drush php-eval "print drush_output_format_properties(drupal_get_complete_schema());" | grep language\.type

gives a nice list

comment.fields.language.type = varchar
field_data_comment_body.fields.language.type = varchar
field_revision_comment_body.fields.language.type = varchar
field_data_body.fields.language.type = varchar
field_revision_body.fields.language.type = varchar
field_data_field_tags.fields.language.type = varchar
field_revision_field_tags.fields.language.type = varchar
field_data_field_image.fields.language.type = varchar
field_revision_field_image.fields.language.type = varchar
node.fields.language.type = varchar
date_format_locale.fields.language.type = varchar
users.fields.language.type = varchar

I start working on alpha order

drush php-eval "print drush_output_format_properties(drupal_get_complete_schema(TRUE));" | grep language\.type | grep field_ | sort

field_data_body.fields.language.type = varchar
field_data_comment_body.fields.language.type = varchar
field_data_field_image.fields.language.type = varchar
field_data_field_tags.fields.language.type = varchar
field_revision_body.fields.language.type = varchar
field_revision_comment_body.fields.language.type = varchar
field_revision_field_image.fields.language.type = varchar
field_revision_field_tags.fields.language.type = varchar

which leaves out

comment.fields.language.type = varchar
date_format_locale.fields.language.type = varchar
locales_target.fields.language.type = varchar
node.fields.language.type = varchar
users.fields.language.type = varchar

which are covered by other issues.

First stab @ field_sql_storage , field.multilingual and locale.module. Let's see how much dies.

Fixed the text from #5

Another stab ... changed all(?) code occurences 'language' and ->language for field and field_ui

There are a lot of langcode related strings and function names still left for change.

[edit]
Following modules use field alteration of some kind

grep -rl "_field_.*_alter" * | cut -c 1-120
core/modules/field/field.api.php
core/modules/field/field.attach.inc
core/modules/field/field.module
core/modules/field/field.multilingual.inc
core/modules/field/modules/list/list.module
core/modules/field/modules/options/options.module
core/modules/field/tests/field.test
core/modules/field/tests/field_test.module
core/modules/field/tests/field_test.storage.inc
core/modules/field_ui/field_ui.test
core/modules/locale/locale.module
core/modules/node/node.module
core/modules/node/node.test
core/modules/poll/poll.module
core/modules/rdf/rdf.module
core/modules/taxonomy/taxonomy.module
core/modules/user/user.module

Failure of TextTranslationTestCase was my bad ... that test case is test a node/add/article

Failure of LanguageUpgradePathTestCase needs some work as the column is changed from language to langcode.
Same goes for FilledUpgradePathTestCase

Failure of EntityFieldQueryTestCase was my bad ...

I expect now only failure of both upgrade path test.

Next step is to http://drupal.org/node/1429136 (UpgradePath test cases)

Hmmm ... EntityFieldQueryTestCase still fails :(

One down ... three tests to fix.

I failed so solve EntityFieldQueryTestCase completely (6 out of 8 are solved)

drush test-run EntityFieldQueryTestCase
Entity query 116 passes, 2 fails, and 0 exceptions
Test EntityFieldQueryTestCase->testEntityFieldQueryMetaConditions() failed: Test with a grouped language meta condition (empty result set).
Test EntityFieldQueryTestCase->testEntityFieldQueryMetaConditions() failed: Test with a grouped delta + language meta condition (empty result set, language condition unsatisifed).

Checking the documentation for these test give me no clue :(

We still need a function field_update_8000() { to change the language column into a langcode column.

I'm puzzled how to alter the schema for this unknown amount of fields having a language column. A code example would be welcome ;)

This needs API Change record for contrib modules to use langcode instead of language.

Updated issue summary.

I think we can do that upgrade path for the mysql field storage engine only. Afaik amitai came up with some working code for that over at #1319040: Remove "target_type" column from db (d7) in entity reference, which could serve as guidance. I don't think we should rely upon field_info_fields() though, but better use low-level functions for retrieving info about existing fields.

@fago tnx

mysql field storage engine only

What about other storage engine? That sounds like a D8 release blocker is it?

Not using field_info_fields was what I thought too.

The link #1319040: Remove "target_type" column from db patch from #37 was useful.

I guess we could use code like field_read_fields without invoking modules.

I give it a try this week(end).

I added a function field_sql_storage_update_8000(&$sandbox) { That seems to be the best place.

@fago
I'm still concerned with "We only upgrade mysql". Please elaborate on that.

Let's see what the test bot says.

Nice ... compared to #12 we have 2 exceptions down.

The failures are in

Entity query (EntityFieldQueryTestCase) [Entity API]	138	2	0
					
Message	Group	Filename	Line	Function	Status
Test with a grouped language meta condition (empty result set).	Other	entity_query.test	1165	EntityFieldQueryTestCase->testEntityFieldQueryMetaConditions()	
Test with a grouped delta + language meta condition (empty result set, language condition unsatisifed).	Other	entity_query.test	1192	EntityFieldQueryTestCase->testEntityFieldQueryMetaConditions()

which reads garble to me. The code that is :(

So some guides needed still.

These tests needs way more info.

   $query = new EntityFieldQuery();
    $query
      ->entityCondition('entity_type', 'test_entity', '=')
      ->fieldCondition($this->fields[0], 'value', 0, '=', NULL, 'group')
      ->fieldLanguageCondition($this->fields[0], LANGUAGE_NONE, '<>', NULL, 'group');
    $this->assertEntityFieldQuery($query, array(), t('Test with a grouped language meta condition (empty result set).'));

and

    $query = new EntityFieldQuery();
    $query
      ->entityCondition('entity_type', 'test_entity', '=')
      ->fieldCondition($this->fields[0], 'value', 0, '=', 'delta', 'language')
      ->fieldDeltaCondition($this->fields[0], 1, '<', 'delta', 'language')
      ->fieldLanguageCondition($this->fields[0], LANGUAGE_NONE, '<>', 'delta', 'language');
    $this->assertEntityFieldQuery($query, array(), t('Test with a grouped delta + language meta condition (empty result set, language condition unsatisifed).'));

I reverted the language_group arguments back to 'language'

Added extra debug info on assertEntityFieldQuery failure.

The 2 failures are still here so this needs help :(

#21: field-language-rename-1439692-21.patch queued for re-testing.

@clemens.tolboom:

I'm still concerned with "We only upgrade mysql". Please elaborate on that.

AFAIK we support other storage engines only on the API level, we don't account for them in update functions as it would be nearly impossible to avoid hook invocations, which in this context are BAD!

+++ b/core/modules/field/field.api.php
@@ -1421,7 +1421,7 @@ function hook_field_language_alter(&$display_language, $context) {
+    locale_field_language_fallback($display_language, $context['entity'], $context['langcode']);

+++ b/core/modules/field/field.attach.inc
@@ -1174,7 +1174,7 @@ function field_attach_view($entity_type, $entity, $view_mode, $langcode = NULL)
+  $options = array('langcode' => $display_language);

+++ b/core/modules/field/tests/field.test
@@ -2790,7 +2790,7 @@ class FieldTranslationsTestCase extends FieldTestCase {
+      $options['langcode'][$id] = field_language($entity_type, $entity, NULL, $display_language);
...
@@ -2810,7 +2810,7 @@ class FieldTranslationsTestCase extends FieldTestCase {

@@ -2810,7 +2810,7 @@ class FieldTranslationsTestCase extends FieldTestCase {
     $grouped_results = _field_invoke_multiple('test_op_multiple', $entity_type, $entities, $null, $null, $options);
     foreach ($grouped_results as $id => $results) {
       foreach ($results as $langcode => $result) {
-        $this->assertTrue(isset($options['language'][$id]), t('The result language %language for entity %id was correctly suggested (display language: %display_language).', array('%id' => $id, '%language' => $langcode, '%display_language' => $display_language)));
+        $this->assertTrue(isset($options['langcode'][$id]), t('The result language %language for entity %id was correctly suggested (display language: %display_language).', array('%id' => $id, '%language' => $langcode, '%display_language' => $display_language)));

+++ b/core/modules/field/tests/field_test.module
@@ -116,7 +116,7 @@ function field_test_field_available_languages_alter(&$languages, $context) {
+    locale_field_language_fallback($display_language, $context['entity'], $context['langcode']);

+++ b/core/modules/locale/locale.module
@@ -325,7 +325,7 @@ function locale_field_language_alter(&$display_language, $context) {
+    locale_field_language_fallback($display_language, $context['entity'], $context['langcode']);

Actually also the display language is a display language code :)

+++ b/core/modules/field/field.api.php
@@ -1732,20 +1732,20 @@ function hook_field_storage_write($entity_type, $entity, $op, $fields) {
+          ->condition('langcode', $languages, 'IN')
...
+          ->condition('langcode', $languages, 'IN')

+++ b/core/modules/field/field.attach.inc
@@ -202,7 +202,7 @@ function _field_invoke($op, $entity_type, $entity, &$a = NULL, &$b = NULL, $opti
+      $languages = _field_language_suggestion($available_languages, $options['langcode'], $field_name);

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.module
@@ -395,20 +395,20 @@ function field_sql_storage_field_storage_write($entity_type, $entity, $op, $fiel
+          ->condition('langcode', $languages, 'IN')
...
+          ->condition('langcode', $languages, 'IN')

+++ b/core/modules/field/tests/field_test.storage.inc
@@ -136,7 +136,7 @@ function field_test_field_storage_write($entity_type, $entity, $op, $fields) {
+          if ($row->type == $entity_type && $row->entity_id == $id && in_array($row->langcode, $languages)) {

I guess $languages should become $langcodes.

+++ b/core/modules/field/field.attach.inc
@@ -319,7 +319,7 @@ function _field_invoke_multiple($op, $entity_type, $entities, &$a = NULL, &$b =
+        $language = !empty($options['langcode'][$id]) ? $options['langcode'][$id] : $options['langcode'];

$language should be $langcode. In this light $available_languages should become $available_langcodes.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,36 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+    $fields = field_read_fields(array(), array('include_deleted' => TRUE, 'include_inactive' => TRUE));

I think we want _update_7000_field_read_fields() here to avoid hook invocations.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.module
@@ -623,8 +623,8 @@ function _field_sql_storage_query_field_conditions(EntityFieldQuery $query, Sele
+    // Add delta / langcode group conditions.

"langcode conditions": do we really want to go this far?

26 days to next Drupal core point release.

I've applied the suggestion from @plach except the hook_update_8000 as that lies outside my comfort zone right now.

function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,

I also changed text by changing language when langcode is meant afaik. This needs a review.

See inter-diff.

+++ b/core/modules/field/field.api.php
@@ -1406,10 +1406,10 @@ function hook_field_attach_prepare_translation_alter(&$entity, $context) {
+ * This hook is invoked to alter the array of display language codes for the given

+++ b/core/modules/field/field.multilingual.inc
@@ -84,10 +84,10 @@ function field_language_delete() {
+ * language codes will be returned, otherwise only LANGUAGE_NONE will be returned.

@@ -103,36 +103,36 @@ function field_language_delete() {
+ * Process the given language code suggestion based on the available language codes.

@@ -227,9 +227,9 @@ function field_has_translation_handler($entity_type, $handler = NULL) {
+ * Otherwise, it returns the current, global language code; or the site's default

@@ -249,15 +249,15 @@ function field_valid_language($langcode, $default = TRUE) {
+ * Returns the display language code for the fields attached to the given entity.
...
+ * requested language code and the actual data available in the fields themselves.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.test
@@ -95,15 +95,15 @@ class FieldSqlStorageTestCase extends DrupalWebTestCase {
+    // Add a translation in an unavailable language code and verify it is not loaded.
     $eid = $evid = 1;
-    $unavailable_language = 'xx';

Comments do not wrap at column 80.

+++ b/core/modules/field/field.attach.inc
@@ -316,12 +316,12 @@ function _field_invoke_multiple($op, $entity_type, $entities, &$a = NULL, &$b =
+        $available_langcodes = field_available_languagecodes($entity_type, $field);

This should be field_available_languages(). I'm wondering how the hell is possible for this code not to fail tests.

+++ b/core/modules/field/field.multilingual.inc
@@ -103,36 +103,36 @@ function field_language_delete() {
+    $drupal_static_fast['field_languagecodes'] = &drupal_static(__FUNCTION__);

Here and below field_languagecodes should be field_langcodes.

+++ b/core/modules/field/field.multilingual.inc
@@ -142,28 +142,28 @@ function field_available_languages($entity_type, $field) {
+    // If we have a language code suggestion and the suggested language code
+    // is available, we return only it.

'is' shoud go one line up: comments should wrap as close as possible to column 80.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,36 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+    $fields = field_read_fields(array(), array('include_deleted' => TRUE, 'include_inactive' => TRUE));

As I wrote previously field_read_fields should be safely replaceable with a _update_7000_field_read_fields.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,36 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+        // Drop indexes
...
+        // Alter field
...
+        // Reinstall indexes

Missing trailing dots.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.module
@@ -386,39 +386,39 @@ function field_sql_storage_field_storage_write($entity_type, $entity, $op, $fiel
+    $all_languagecodes = field_available_languages($entity_type, $field);
+    $field_languagecodes = array_intersect($all_languagecodes, array_keys((array) $entity->$field_name));
...
+      $langcodes = !empty($entity->$field_name) ? $field_languagecodes : $all_languagecodes;
...
+    foreach ($field_languagecodes as $langcode) {

+++ b/core/modules/field/tests/field_test.storage.inc
@@ -126,17 +126,17 @@ function field_test_field_storage_write($entity_type, $entity, $op, $fields) {
+    $all_languagecodes = field_available_languages($entity_type, $field);
+    $field_languagecodes = array_intersect($all_languagecodes, array_keys((array) $entity->$field_name));

@@ -150,7 +150,7 @@ function field_test_field_storage_write($entity_type, $entity, $op, $fields) {
+    foreach ($field_languagecodes as $langcode) {

languagecodes -> langcodes

+++ b/core/modules/field/tests/field_test.storage.inc
@@ -126,17 +126,17 @@ function field_test_field_storage_write($entity_type, $entity, $op, $fields) {
+      $languages = !empty($entity->$field_name) ? $field_languagecodes : $all_languagecodes;
...
+          if ($row->type == $entity_type && $row->entity_id == $id && in_array($row->langcode, $languages)) {

$languages -> $langcodes
$all_languagecodes -> $all_langcodes

+++ b/core/modules/locale/locale.module
@@ -340,23 +340,23 @@ function locale_field_language_alter(&$display_language, $context) {
+  foreach ($display_langcode as $field_name => $field_language) {

field_langcode

+++ b/core/modules/locale/locale.module
@@ -365,7 +365,7 @@ function locale_field_language_fallback(&$display_language, $entity, $langcode)
       foreach ($fallback_candidates as $fallback_language) {
         if (isset($entity->{$field_name}[$fallback_language])) {
-          $display_language[$field_name] = $fallback_language;
+          $display_langcode[$field_name] = $fallback_language;
           break;

fallback_language -> fallback_langcode

21 days to next Drupal core point release.

This should be field_available_languages(). I'm wondering how the hell is possible for this code not to fail tests.

This is indeed interesting. There seems to be a problem, maybe the tests are not completed.

Here is a new patch which reroles the patch and fixes the issues from above.

There is another patch for the testbot.

Reviewing...

Open points still stainding according franskuipers and clemens.tolboom

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,36 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,

We should delete this function.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,36 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+    $fields = _update_7000_field_read_fields();

This is a Drupal 7 function which we should not use.

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,36 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+    drupal_load('module', 'field_sql_storage');

Is this ok to use?

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,36 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+        $schema += _field_sql_storage_schema($field);

Is this hook-less?

To test the upgrade path I extended the node type "Article" to have two instances (single & multivalue) of every core field type and generated 50 nodes with content.
The only issue I cam across was the fact that the scheme wasn't rebuild after the update and thus the site had a WSDO until a full cache flush was done.
Adding a drupal_get_schema(NULL, TRUE); after the schema update solves this issue.

We (FransKuipers and me) rewrote field_sql_storage_update_8000 which is now without hook invocation.

Please review :)

function field_sql_storage_update_8000(&$sandbox) {
    if (db_table_exists('field_config')) {
    $result = db_select('field_config', 'fc')
      ->fields('fc', array('field_name'))
      ->condition('storage_type', 'field_sql_storage')
      ->execute()
    ;

    $primary_key = array (
      'entity_type',
      'entity_id',
      'deleted',
      'delta',
      'langcode',
    );
    $langcode_index = array(
      'langcode',
    );
    $field_langcode = array(
      'type' => 'varchar',
      'length' => 32,
      'not null' => true,
      'default' => '',
    );

    foreach ($result as $row) {
      $field_name = $row->field_name;
      $tables = array(
        'field_data_' . $field_name,
        'field_revision_' . $field_name,
      );
      foreach ($tables as $table) {
        db_drop_primary_key($table);
        db_drop_index($table, 'language');
        db_change_field($table, 'language', 'langcode', $field_langcode);
        db_add_primary_key($table, $primary_key);
        db_add_index($table, 'langcode', $langcode_index);
      }
    }
  }

We commented out

  // Force a schema rebuild.
  //drupal_get_schema(NULL, TRUE);

as it's unclear to us _AND_ it involves invoking hook_schema.

We fixed the _revision primary key.

Tested the patch again and this time the explicit rebuild of the schema cache wasn't necessary :)
Thus I removed the lines I added earlier.

+++ b/core/modules/entity/tests/entity_query.testundefined
@@ -1526,7 +1526,9 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
+      if (!$this->assertEqual($results, $intended_results, $message)) {
+        debug($results, $message);

Just a leftover debug statement

+++ b/core/modules/field/field.multilingual.incundefined
@@ -104,36 +104,37 @@ function field_language_delete() {
+      drupal_alter('field_available_languages', $langcodes, $context);

+++ b/core/modules/field/field.multilingual.incundefined
@@ -281,38 +284,38 @@ function field_valid_language($langcode, $default = TRUE) {
+      drupal_alter('field_language', $display_langcode, $context);

I'm wondering whether it's intended to rename the hooks and some other functions as well.
Just from the perspective of a module developer i would though preper the current version.

I read the full big patch and didn't saw any problem with the renaming of language to langcode.

@dawehner good catch for the debug.

But are we changing the hooks? We only changed the hook parameter names from language* to langcode* right? Or do I miss something? One could argument the hook names should be changed too.

Thanks for the checks :)

I don't think we want to change also hooks and function names, but we should hear Gabor's voice on this.

+++ b/core/modules/field/field.multilingual.inc
@@ -104,36 +104,37 @@ function field_language_delete() {
-      drupal_alter('field_available_languages', $languages, $context);
-      $field_languages[$entity_type][$field_name] = $languages;
+      drupal_alter('field_available_languages', $langcodes, $context);

+++ b/core/modules/field/field.multilingual.inc
@@ -281,38 +284,38 @@ function field_valid_language($langcode, $default = TRUE) {
-      drupal_alter('field_language', $display_language, $context);
+      drupal_alter('field_language', $display_langcode, $context);

Attention: Only the name of the variables, used for hook invocation, are changed. There's no such thing as a renamed hook.

+++ b/core/modules/entity/tests/entity_query.testundefined
@@ -1526,7 +1526,9 @@ class EntityFieldQueryTestCase extends DrupalWebTestCase {
+      if (!$this->assertEqual($results, $intended_results, $message)) {
+        debug($results, $message);

As this debug call is conditional it might make even sense to keep it (at least it doesn't seem to harm a thing), but I don't know what the rules for such constructs are.

@das-peter: let's keep the debug then.

@Gábor Hojtsy: Who must review this to get this RTBC?

@plach could you please review the last hurdle mentioned by you in #25 + #28 and resolved similar to #35 in patch #37 concerning

function field_sql_storage_update_8000(&$sandbox) {

I think #38 is then RTBC :)

(as we keep the debug and don't change hooks #39 - #44 )

I fixed some language occurence left. I also rewrote the update function to use the (supposedly?) proper update API function. We really need @catch to have a look to it sooner or later. Maybe we should assign this to him when this is RTBC (and now IMO it is).

+++ b/core/modules/field/field.attach.inc
@@ -316,12 +316,12 @@ function _field_invoke_multiple($op, $entity_type, $entities, &$a = NULL, &$b =
+        $language = !empty($options['langcode'][$id]) ? $options['langcode'][$id] : $options['langcode'];
+        $langcodes = _field_language_suggestion($available_langcodes, $language, $field_name);

$language => $langcode

+++ b/core/modules/field/field.multilingual.inc
@@ -104,36 +104,37 @@ function field_language_delete() {
  * @param $language_suggestion

$langcode_suggestion

+++ b/core/modules/field/field.multilingual.inc
@@ -143,28 +144,28 @@ function field_available_languages($entity_type, $field) {
+    // If we have a language suggestion and the suggested language is
+    // available, we return only it.

@@ -229,9 +230,9 @@ function field_has_translation_handler($entity_type, $handler = NULL) {
+ * Otherwise, it returns the current, global language code; or the site's default

Wrong comment wrapping.

+++ b/core/modules/field/tests/field.test
@@ -2935,48 +2935,48 @@ class FieldTranslationsTestCase extends FieldTestCase {
+    $requested_langcodes = $enabled_langcodes[0];
+    $display_langcodes = field_language($entity_type, $entity, NULL, $requested_langcodes);

Here and below $requested_langcodes should be singular, as only a single language is requested.

+++ b/core/modules/locale/locale.module
@@ -365,32 +365,32 @@ function locale_field_language_alter(&$display_language, $context) {
+ * @param $field_langcode

If we want to make this change, this should be plural here and below.

-4 days to next Drupal core point release.

Weird, I manually tested the upgrade path and it completed nicely...

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,51 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+      db_add_primary_key($table, $primary_key_data);

Hmm, here is the culprit :)

Attached a working patch plus interdiff with #38.

-4 days to next Drupal core point release.

Gábor Hojtsy:

Since the only part that still needs review is the update function and no one here seems to be comfortable with reviewing it, what about marking this RTBC and assigning it to @catch so he can provide us any feedback about possible pending concerns?

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,52 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+  // Retrieve field data.
+  $fields = _update_7000_field_read_fields(array('storage_type' => 'field_sql_storage'));
+

This function (name) may never be used for D8. AFAIK it is a helper function for D6 to D7 upgrades.

That's why I used the db_query.

    if (db_table_exists('field_config')) {
    $result = db_select('field_config', 'fc')
      ->fields('fc', array('field_name'))
      ->condition('storage_type', 'field_sql_storage')
      ->execute()
    ;

+++ b/core/modules/field/field.multilingual.inc
@@ -152,8 +152,8 @@ function _field_language_suggestion($available_langcodes, $langcode_suggestion,
+    // If we have a language suggestion and the suggested language is available,
+    // we return only it.

Should this be langcode as well?
'If we have a langcode suggestion and the suggested langcode is available,'

+++ b/core/modules/field/tests/field.test
@@ -2945,8 +2945,8 @@ class FieldTranslationsTestCase extends FieldTestCase {
-    $requested_langcodes = $enabled_langcodes[0];
-    $display_langcodes = field_language($entity_type, $entity, NULL, $requested_langcodes);
+    $requested_langcode = $enabled_langcodes[0];
+    $display_langcodes = field_language($entity_type, $entity, NULL, $requested_langcode);

Good catch over the plural form.

I guess $display_langcodes should now also singular instead of plural.

@Gábor Hojtsy: He ... we past each others comment :)

It could be good to transform that function but should we do it for this issue? I somehow guess yes.

If we do that it needs a test as I think its data column contains values also available through the individual columns. Not 100% sure.

function _update_7000_field_read_fields(array $conditions = array(), $key = 'id') {
...
  foreach ($query->execute() as $record) {
    $field = unserialize($record['data']);
...

Fixed the language suggestion comment and another 'language' occurrence.

That's why I used the db_query.

I recall from the deadly #1164852: Inconsistencies in field language handling that it's absolutely discouraged to directly query field_config, since we cannot rely on it being always there. I really think using _update_7000_field_read_fields() is the right thing to do here, since it encapsulates the field retrieval logic, and no, I don't think renaming it from 7000 to 8000 belongs to an issue dealing with language code renamings. My point is we should really ask someone knowledgeable, as @catch.

@plach: as you put it that way we certainly have to use a function like that :)

+++ b/core/modules/field/modules/field_sql_storage/field_sql_storage.install
@@ -77,3 +77,52 @@ function _update_7000_field_sql_storage_write($entity_type, $bundle, $entity_id,
+  $fields = _update_7000_field_read_fields(array('storage_type' => 'field_sql_storage'));

So we agree this function should be reused by renaming it to _update_8000_field_read_fields or maybe a better name as @Gábor Hojtsy suggested too.

What about adding hookless into it's name and make it public. It is now used only by hook_update_8000 but is that it's only use case?

It is similar to http://api.drupal.org/api/drupal/core!modules!field!field.crud.inc/funct... right?

function field_read_fields_hookless()
function _field_read_fields_hookless()

Sounds as good plan but for the life of me I don't get why we have to discuss it here, however the update prefix means exactly hookless. I don't think we need a new terminology.

@plach : You just want to use the function whatever it's name. Sounds like a brilliant plan :) ... or I completely misunderstood.

@Gábor Hojtsy guess we should follow along @plach suggestion from #51 and mark this as RTBC and assign to catch.

(OT)

@clemens.tolboom:

Trying to explain myself better: having a better name for that function, making clear it's an API function pertaining the upgrade path sounds like a good plan. I don't think the hookless terminology is something we need to introduce since we already have the "update_*" prefix.

There is a bunch of _7000 functions in field.install, not mentioning another one in node.install and one in field_sql_storage.install. None of them is used, should we rename them all for consistency?

Here it is.

Committed to 8.x. Thanks!

@catch:

Would you please confirm field_sql_storage_update_8000() is ok?

No it's not quite right, there's no reason to do this since the schema for field config hasn't changed, those functions are tied to schema version, not API version.

-function _update_7000_field_read_fields(array $conditions = array(), $key = 'id') {
+function _update_8000_field_read_fields(array $conditions = array(), $key = 'id') {

See also #1134088: Properly document update-api functions.

@catch so we shouldn't have renamed the _update_7000_field_read_fields ?

Leave it 'as is' as the schema we are querying is still a D7 schema? Makes sense.

So all _7000_ functions are meant to work on a D7 scheme in a D8 context.

(I've read and commented on #1134088: Properly document update-api functions)

@ Gábor Hojtsy: yes anything named 8000 that's not specifically related to a Drupal 8 schema change is named incorrectly, that's discussed in #1239370: Module updates of new modules in core are not executed which is another pile of evil :(

I've committed/pushed the follow-up, thanks!

OK, committed that one too.