"Content translation: Translations" relationship broken

This was changed as a fix to this issue: #1940212: "Content translation: Translations" relationship incorrectly specifying translation source.

I am seeing an issue as well where all nodes without any translations are being joined to one another because they all have a default tnid of '0'. The node table should not be allowed to join to itself where the tnid field is equal to zero. Attached is a patch that adds an additional condition on the join.

@Mav-im, please see if the attached patch fixes your issue.

Drupal's notion of a translation set -- defaulting to a tnid of zero -- is quite a pain here. This would be much easier if all untranslated nodes simply had their own nid as their tnid. This is further complicated by #2367939: Translation sets should be 'removed' when they contain only one node, incidentally. (Or simplified by it, depending on your view point; but it's another inconsistency at any rate.)

Now we certainly don't want to join on arbitrary untranslated nodes; but we also need to ensure that the relationship does not fail entirely when there is only one node in the translation set. If tnid is zero, we still need to obtain the row for the original node.

e.g. If we're starting with an English node, then using the relationship to obtain all translations and filter to the translation for user's language, and it turns out that the user's language is English and the node has not been translated yet, returning no rows would be a bad thing.

The join presumably needs to be something like the following?

{node} original
INNER JOIN {node} translation
ON (original.tnid > 0 AND translation.tnid = original.tnid)
    OR (original.tnid = 0 AND translation.nid = original.nid)

@jweowu,

You are absolutely correct and this is exactly what my patch accomplishes. It adds the condition on the 'JOIN' clause, not on the entire SQL statement, so the original node is returned unless the user specifically requires the relationship to exist, in which case an INNER join will be used instead of an OUTER JOIN.

I've tested the patch, and I'm afraid that it exhibits the very problem I indicated.

In the following, node_node is all of the nodes in the same translation set as node_field_data_field_promotion_reference (also being a node), which we then filter to the specific translation for the language in question.

The original join was:

INNER JOIN node node_node
ON node_field_data_field_promotion_reference.tnid = node_node.tnid
AND node_node.language = 'en'

Which is wrong (as per this issue).

With the patch, the revised join is now:

INNER JOIN node node_node
ON node_field_data_field_promotion_reference.tnid = node_node.tnid
AND (node_node.tnid != 0
     AND node_node.language = 'en')

Which is also wrong. As per my previous reply, this join needs to be equivalent to the following.

INNER JOIN node node_node
ON ((node_field_data_field_promotion_reference.tnid > 0
     AND node_node.tnid = node_field_data_field_promotion_reference.tnid)
    OR (node_field_data_field_promotion_reference.tnid = 0
        AND node_node.nid = node_field_data_field_promotion_reference.nid))
AND node_node.language = 'en'

Again, this is because in the scenario where there is only a single node in the would-be translation set, that node will have a tnid of zero, so your tnid != 0 condition excludes it. If the tnid is zero, the join has to be on the node ID.

The full query in my test was:

SELECT field_data_field_promotion_reference.delta AS field_data_field_promotion_reference_delta, node_node.nid AS node_node_nid
FROM
node node
LEFT JOIN field_data_field_promotion_reference field_data_field_promotion_reference ON node.nid = field_data_field_promotion_reference.entity_id AND field_data_field_promotion_reference.entity_type = 'node'
INNER JOIN node node_field_data_field_promotion_reference ON field_data_field_promotion_reference.field_promotion_reference_target_id = node_field_data_field_promotion_reference.nid
INNER JOIN node node_node ON node_field_data_field_promotion_reference.tnid = node_node.tnid AND (node_node.tnid != 0 AND node_node.language = 'en')
WHERE (( (node.nid = '108681' ) )AND(( (node_node.status = '1') )))
ORDER BY field_data_field_promotion_reference_delta ASC;

Which is from the following exported View (which I'll describe, as it won't be directly importable).

The view takes a Node ID argument (automatically obtained from the path for the page being viewed, so the argument represents the current page). There are two relationships.

The first relationship obtains node IDs from an entity reference field on the argument's node (each page may reference some 'promotion' nodes to be displayed. The precise promotion node IDs being referenced are not necessarily for the same language as the page which is referencing them, however).

The second relationship is the Translations relationship -- it's required, and it obtains the node translation for each referenced promotion in the current user's language.

We format the results as Content, specifying the Translations relationship (i.e. rendering the language-appropriate set of promotions for the current page).

(We also filter them by published status, and sort them on the delta value from the entity reference field).

So to adapt the example from my previous reply, if the current page references promotion node ID 50, and node 50 is an untranslated English promotion node, and the user's language is English, then we should be rendering that promotion (as it is in the correct language). But because its tnid is zero, no results will be found with this patch.

$view = new view();
$view->name = 'referenced_promotions';
$view->description = 'Testing';
$view->base_table = 'node';
$view->human_name = 'Referenced promotions';
$view->core = 7;
$view->api_version = '3.0';
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */

/* Display: Master */
$handler = $view->new_display('default', 'Master', 'default');
$handler->display->display_options['use_more_always'] = FALSE;
$handler->display->display_options['access']['type'] = 'perm';
$handler->display->display_options['cache']['type'] = 'none';
$handler->display->display_options['query']['type'] = 'views_query';
$handler->display->display_options['query']['options']['pure_distinct'] = TRUE;
$handler->display->display_options['exposed_form']['type'] = 'basic';
$handler->display->display_options['pager']['type'] = 'none';
$handler->display->display_options['pager']['options']['offset'] = '0';
$handler->display->display_options['style_plugin'] = 'default';
$handler->display->display_options['row_plugin'] = 'node';
$handler->display->display_options['row_options']['relationship'] = 'translation';
$handler->display->display_options['row_options']['links'] = FALSE;
/* Relationship: Entity Reference: Referenced Entity */
$handler->display->display_options['relationships']['field_promotion_reference_target_id']['id'] = 'field_promotion_reference_target_id';
$handler->display->display_options['relationships']['field_promotion_reference_target_id']['table'] = 'field_data_field_promotion_reference';
$handler->display->display_options['relationships']['field_promotion_reference_target_id']['field'] = 'field_promotion_reference_target_id';
$handler->display->display_options['relationships']['field_promotion_reference_target_id']['required'] = TRUE;
/* Relationship: Content translation: Translations */
$handler->display->display_options['relationships']['translation']['id'] = 'translation';
$handler->display->display_options['relationships']['translation']['table'] = 'node';
$handler->display->display_options['relationships']['translation']['field'] = 'translation';
$handler->display->display_options['relationships']['translation']['relationship'] = 'field_promotion_reference_target_id';
$handler->display->display_options['relationships']['translation']['required'] = TRUE;
/* Sort criterion: Content: Promotion reference (field_promotion_reference:delta) */
$handler->display->display_options['sorts']['delta']['id'] = 'delta';
$handler->display->display_options['sorts']['delta']['table'] = 'field_data_field_promotion_reference';
$handler->display->display_options['sorts']['delta']['field'] = 'delta';
/* Contextual filter: Content: Nid */
$handler->display->display_options['arguments']['nid']['id'] = 'nid';
$handler->display->display_options['arguments']['nid']['table'] = 'node';
$handler->display->display_options['arguments']['nid']['field'] = 'nid';
$handler->display->display_options['arguments']['nid']['default_action'] = 'default';
$handler->display->display_options['arguments']['nid']['default_argument_type'] = 'node';
$handler->display->display_options['arguments']['nid']['summary']['number_of_records'] = '0';
$handler->display->display_options['arguments']['nid']['summary']['format'] = 'default_summary';
$handler->display->display_options['arguments']['nid']['summary_options']['items_per_page'] = '25';
$handler->display->display_options['arguments']['nid']['specify_validation'] = TRUE;
/* Filter criterion: Content: Published */
$handler->display->display_options['filters']['status']['id'] = 'status';
$handler->display->display_options['filters']['status']['table'] = 'node';
$handler->display->display_options['filters']['status']['field'] = 'status';
$handler->display->display_options['filters']['status']['relationship'] = 'translation';
$handler->display->display_options['filters']['status']['value'] = '1';
$handler->display->display_options['filters']['status']['expose']['operator_id'] = '';
$handler->display->display_options['filters']['status']['expose']['label'] = 'Published';
$handler->display->display_options['filters']['status']['expose']['operator'] = 'status_op';
$handler->display->display_options['filters']['status']['expose']['identifier'] = 'status';
$handler->display->display_options['filters']['status']['expose']['required'] = TRUE;

$translatables['referenced_promotions_test'] = array(
  t('Master'),
  t('more'),
  t('Apply'),
  t('Reset'),
  t('Sort by'),
  t('Asc'),
  t('Desc'),
  t('Translations'),
  t('All'),
  t('Published'),
  t('View panes'),
);

@jweowu,

All I see in your join statements are 'INNER JOIN' which means you are 'requiring' the relationship to exist. This feature is meant to filter out rows where there is no record on the right side of the join. If you don't require the relationship to exist, a 'LEFT JOIN' will be used instead and will keep all records on the left side regardless if there is a matching record on the right side! Here is the SQL output from my view with the relationship not required:

FROM 
{node} node
LEFT JOIN {node} node_node ON node.tnid = node_node.tnid AND (node_node.tnid != 0 AND node_node.language = 'en')

Please test with the relationship not required!

I feel that we're talking at cross purposes.

If node 50 is an English node, and the relationship is looking for the English node in the translation set for node 50, then node 50 should be returned regardless of whether the relationship is "required" or not -- because the relationship does, in fact, exist: Node 50 is the English node in its own translation set, even if that set comprises only that single node (in which case tnid is zero).

The only way in which the "not required" option should ever come into play with Translations is if the relationship is constrained to a language for which no translation exists.

I was noticing that adding this relationship was causing every node to be joined to every other node when the relationship is not required and you select "All" from Translation options. I am using Views 7.x-3.13.

Based on @jweowu's suggested join:

{node} original
INNER JOIN {node} translation
ON (original.tnid > 0 AND translation.tnid = original.tnid)
    OR (original.tnid = 0 AND translation.nid = original.nid)

I think this would be equivalent:

{node} original
INNER JOIN {node} translation
ON (translation.tnid != 0)
    OR (original.tnid = 0 AND translation.nid = original.nid)

So the current patch is missing the OR (original.tnid = 0 AND translation.nid = original.nid) part. However, while looking at helping with a patch, I don't know how to reference the original alias in the formula. I've attached a patch that does what I think we want, but it is kinda ugly.

If we want to remove the I think it would require adding another placeholder in SelectQuery::addJoin(). Unless there is another way to achieve the grouping that I'm not aware of?

I'm also not sure about what the behavior used to be or whether or not this is an API change, which @Mav-im brings up, and still needs to be considered.

Also, in case anybody else is using Views 7.x-3.13 and gets an exception, you need to update to the latest dev or apply the patch in #1090432-22: Allow additional field-to-field conditions with the default views_join handler

@jweowu,

I missed that you were talking about joining to the same node. In this case you are correct and there is a bug with my patch.

@Mav-im,

I was also surprised to see this change as it broke our site as well. I agree with you completely on #11. I think a better solution would have been to rename the original relationship to "Source translation" with a deprecated status and create a new "All translations" relationship. It's in the past now so let's focus on fixing the bug, yeah?

@jojonaloha,

I don't think your query below is equivalent to @jweowu's from his comment #4. With your query I don't see how the tables will join if believe the join would be to all nodes where the tnid is not zero including nodes from other translation sets?

{node} original
INNER JOIN {node} translation
ON (translation.tnid != 0)
    OR (original.tnid = 0 AND translation.nid = original.nid)

I don't believe there is a way to reference the left table in the join as the alias is only available for the right table. I believe using $def['left_table'] as your patch does uses the actual table name and not the alias assigned to it via views. This could break if other joins to the node table were part of the query. I think your solution to adding an alias for the left table in SelectQuery::addJoin() is correct, but I'm not sure how easy it will be to get a fix for this in.

If I'm not mistaken, we can write our own 'join' handler that would provide aliases for both sides of the join and instruct the relationship to use that join handler. I believe this is the best way forward at this point.

Mav-im: I agree with that approach. Reverting & renaming the original relationship, and adding a new one for the revised behaviour makes good sense to me.

In #1940212: "Content translation: Translations" relationship incorrectly specifying translation source it just hadn't occurred to me that only obtaining translations if you were starting with the translation set's source node could be a useful behaviour -- it had genuinely seemed like a mistake.

It's certainly bad to have broken views which wanted it to work that way (ignoring for a moment my "join on tnid=0" blunder in the would-be fix, which I had missed on account of the case being 100% mitigated in my own testing, and which caused even more breakage), so I'm on board with having distinct relationships for the distinct behaviours -- and giving them unambiguous names and descriptions!

Sorry, you are right, the joins I listed in my previous comment are not equivalent. However, that was just based on @jweowu's example of how to solve it. Here are the actual joins that I'm seeing in a view using this relationship, which is not required and selecting "All" from Translation options.

Before the patch (which does join all the nodes with every other node where tnid=0):

FROM 
{node} node
LEFT JOIN {node} node_node ON node.tnid = node_node.tnid

After the patch:

FROM 
{node} node
LEFT JOIN {node} node_node ON node.tnid = node_node.tnid AND (node_node.tnid != 0 OR (node.tnid = 0 AND node_node.nid = node.nid))

So the result is actually that the extra formula is added as an AND condition to the base condition.

As for the use of $def['left_table'] I could be wrong, but I think that is the alias based on a few lines up (line 56) $def['left_table'] = $this->table_alias;

So I think my patch still applies.

Patch at comment #12 works for me

Latest patch (https://www.drupal.org/node/2638220#comment-10925969) works with latest dev version as expected

Patch #12 doesn't work with the latest release 7.x-3.16. It works properly with the latest dev version.

Patch #12 doesn't work with the latest stable release 7.x-3.18. Please find attached a patch with a similar solution that works with the latest stable releases of Views.

The final query of this patch is the same as patch #12.

Patch #12 worked fine for 7.x-3.15 but not with the latest stable views release 7.x-3.18.

However, patch in #19 works fine with views 7.x-3.18.

Committed. Thank you.