Assumin we have field_ref as reference to another entity, if we do for example:

function foo_node_presave($node) {
  $wrapper = entity_metadata_wrapper('node', $node);
  $wrapper->field_ref[] = 1;
}

Then the node object has $node->field_ref[LANGUAGE_NONE][0]['target_id'] = 1; However, since we don't have the target_type the node won't be saved.

It seems that entity_metadata_field_property_set() doesn't deal with populating the target_type. (Not sure if we should deal with it in Entity API, or here)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Damien Tournoud’s picture

We add the target_type in entityreference_field_presave() hook, so it might be an issue with the execution order of those hooks.

We don't actually use this information anywhere, but it seemed like a good idea to have it in the database. Maybe we should just remove the column altogether?

amitaibu’s picture

> Maybe we should just remove the column altogether?

Since we already have the target_type in the field info, maybe we can remove it. What was the reason in the first place?

Damien Tournoud’s picture

It seemed like a good idea to have it, at least to ease joining. But since we are not going to support referencing multiple entity types (because everything, starting with Entity Metadata and Views, expects to know the entity type beforehand), I agree we should remove it.

amitaibu’s picture

Title: entity_metadata_field_property_set() doesn't work on presave functions » Remove "target_type" column form db

As we don't have API to change the schema -- #937442: Field type modules cannot maintain their field schema (field type schema change C(R)UD is needed) , should the update hook deal with SQL only?

Damien Tournoud’s picture

Yep, the update hook should only deal with field_sql_storage fields. It doesn't hurt if we keep this data for other field storage anyway.

amateescu’s picture

Approximately on-topic: is there a reason why we keep the target_type in the field info instead of the instance info?

I'm asking this because I want to use entityreference as storage for entityqueue (nodequeue rewrite), with queue bundles for every entity type. By not having the target type in instance, I'm kinda forced to create a new field for every entity type.

Damien Tournoud’s picture

@amateescu: it should be doable now that hook_option_lists() gets the $instance, but we need to look at all other issues (integration with Entity Metadata, Views, etc.). Please open a separate issue.

amitaibu’s picture

Category: bug » task
Status: Active » Needs review
FileSize
2.26 KB

Patch attached :)

amitaibu’s picture

Hey guys, can anybody give it a try, so we can get it in? :)

Status: Needs review » Needs work

The last submitted patch, 1319040-remove-target-type.patch, failed testing.

amitaibu’s picture

Status: Needs work » Needs review
FileSize
2.28 KB

Re-rolled.

fago’s picture

Status: Needs review » Needs work

I had problems setting entity reference fields with Rules, got the following error:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'field_uploader_target_type' cannot be null:

The patch from #11 solved it for me though. Patch looks good to me and makes sense, however:

+function entityreference_update_7001() {
+  foreach (field_info_fields() as $field_name => $field) {
+    if ($field['type'] != 'entityreference') {
+      con

I guess we should check we are using sql storage here too?

amitaibu’s picture

@fago, according to #5 I assumed we shouldn't do a special check. Not sure, though.

amitaibu’s picture

Attached patch checks sql_field_storage module exists, and that the field is using it.

Let's get it in, before tagging beta4.

amitaibu’s picture

Status: Needs work » Needs review
fago’s picture

Status: Needs review » Reviewed & tested by the community

Great, looks good. Yep let's get this in, so updating the field works with the wrappers / Rules too.

amitaibu’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

amitaibu’s picture

Ouch, it seems I forgot to update the field's indexes, which causes a WSOD when editing an existing field. Sorry about that :/
I've tested it an committed, to make sure we have as little as possible people effected with the wrong patch.

kmare’s picture

Status: Needs work » Fixed

hello,
when updating with this latest patch, I'm getting the following error updating:

entityreference module
Update #7001

    Failed: DatabaseSchemaObjectExistsException: Cannot add index <em class="placeholder">field_contact_related_company_target_id</em> to table <em class="placeholder">field_data_field_contact_related_company</em>: index already exists. in DatabaseSchema_mysql->addIndex() (line 432 of /var/www/html/mysite/includes/database/mysql/schema.inc).

thank you in advance for any help

JStanton’s picture

Status: Fixed » Needs work

I too got this error, however, the index seems to be in there correctly.

kmare’s picture

Status: Fixed » Needs work

well, it doesn't really work for me. When I edit again the specific field and save it, I get the following error:

Attempt to update field Company failed: Cannot add index <em class="placeholder">field_contact_related_company_target_id</em> to table <em class="placeholder">field_data_field_contact_related_company</em>: index already exists..

which doesn't allow me to edit it. Is there a solution for that or will I just have to delete and recreate that field losing data?
Thank you in advance for any help

amitaibu’s picture

Status: Needs work » Fixed

For those that got the corrupted version (i.e. before the fix in #18), you can run this php snippet, which should fix it. (Don't forget to always backup your db

foreach (field_info_fields() as $field_name => $field) {
  if ($field['type'] != 'entityreference') {
    // Not an entity reference field.
    continue;
  }
  if ($field['storage']['type'] !== 'field_sql_storage') {
    // Field doesn't use SQL storage.
    continue;
  }

  // Update the field settings.
  $field = field_info_field($field_name);
  $field['indexes'] = array('target_id' => array('target_id'));
  field_update_field($field);
}

You can execute this script for example, by enabling the Devel module and going to devel/php

kmare’s picture

Amitaibu,
this is not working for me. It returns the same error. I tried to delete the field that was in reported in the error, but updating again, it returns the same error as before, but this time about the next referenced field.

kmare’s picture

Category: task » bug
Priority: Normal » Major
Status: Fixed » Needs review

hello again (sorry for spamming),
I somehow managed to downgrade to a previous version I fortunately had around and things are ok now, at least with my installation. Of course the problem remains if I want/need to upgrade :( . What I'd like to know is if there's "no turning back" and I'll have to delete and recreate all the entityreference fields to upgrade (losing data of course).
Thanks everyone for your patience.

Status: Needs review » Needs work

The last submitted patch, 1319040-remove-target-type-18.patch, failed testing.

amitaibu’s picture

Status: Needs work » Fixed

> What I'd like to know is if there's "no turning back" and I'll have to delete and recreate all the entityreference fields to upgrade (losing data of course).

Unless you backed up your site before the upgrade, you might need to recreate the field.

John Morahan’s picture

Title: Remove "target_type" column form db » Remove "target_type" column from db
Status: Fixed » Needs review
FileSize
1.41 KB

I got the same error, despite updating from a version before #14 was applied at all.

The problem is not that the index does not exist. The problem is that the index does exist, for whatever reason. So when you try to run the update, it tries to add the index, fails because the index already exists, and throws an exception. It therefore doesn't get a chance to remove the target_type column from any further tables, so when you subsequently try to edit a field that uses one of those tables, you fail, because the target_type column that wasn't removed is NOT NULL.

How about something like this?

kmare’s picture

Thanks Amitaibu and John Morahan! Executing #22 and then applying 1319040.patch at #27 fixed the whole problem without losing data! Of course I had to follow the above sequence, else it would delete all referenced data (at least I had a recent backup :P).
Thanks again guys :)

wroxbox’s picture

#27 fixed for me the same as #21 reported. Thank you.

rwilson0429’s picture

Patch in #27 fixed for me too with no loss of data. I was getting the error in #21 when running update.php, as well as, when attempting to edit the reference field. Tried #22 but, did not work for me, then, I applied patch in #27 and all is well. Thanks.

amitaibu’s picture

@wroxbox, @rwilson0429,

Did you get the error after updating the code to #14 (which was not a proper fix), or did you upgrde from the latest beta?
The thing is that I don't understand how the index already exists.

rwilson0429’s picture

I noticed the error after upgrading to the latest dev and running update.php. The update ended with the error in #19. I tried #14 in an attempt to resolve the problem.

Crell’s picture

Unfortunately, even with the above fix patch this issue causes a variety of fatal errors for any content type that has been pushed to features, because features field exports include the field schema. Even after a complete reinstall that means those features are still fatally broken and require manual resolution.

I should note that Field API specifically is not designed to allow schema changes. Changing the schema anyway is probably going to break even more than I'm running into. (I also had custom code that relied on the target_type column; I was able to convert it to use the $instance instead, but that is still an API break.)

HnLn’s picture

The latest dev completely crashed my site after going through the database update process (all my fields are in features).

rogical’s picture

Crell's right, I'm using features to deploy entity reference fields, always failed to update even with the latest patch.

I posted my issue here: Site crashed when upgrade 7.x-1.0-beta3 to 7.x-1.x-dev

rogical’s picture

I think maybe we can use Features Tools to update features files in update.

amitaibu’s picture

FileSize
2.55 KB

Here's a proposition (work in progress -- need to add more docs). We have a simple "migrate" that allows users to update the scema, after they manually changed their features export. We can remove this migrate before the official release.

Damien Tournoud’s picture

Status: Needs review » Fixed

I committed a fix to make the upgrade path a tad more robust. We don't support updates to -dev versions, so there is no reason to commit anything else here.

Features are going to need to be fixed manually. There is no way around that. Features should *not* save the schema and index definition. It's a bug in Features, nothing we can fix in Entity Reference.

Pedro Lozano’s picture

As I noted in #1438922: Crash when saving a field after an update to the latest -dev saving the settings for an old field still gets the crash.

I know you don't support upgrades to -dev versions but just an additional field_update_field() after updating the sql schema solves this problem for old fields.

I posted a patch in the other issue with a new upgrade function in case you want to use it.

Status: Fixed » Closed (fixed)

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