Use leftJoin when base table uses a leftJoin [#1422040]

I discovered this bug after updating Views recently to 7.x-3.1. Some of my views were not showing all the results that I expected. After looking at the query, I saw that it is because the Views have some joins that are not required (IE: left joins instead of inner joins), but domain_alter_node_query() always does an inner join.

I am using Drupal 7.8 and Domain Access 7.x-2.18.

Comment	File	Size	Author
#8	domain-node_access_joins-1422040-8.patch	3.08 KB	jojonaloha
#5	domain-use_left_joins-1422040-example_view.txt	26.19 KB	jojonaloha
#3	domain-use_left_joins-1422040-3.patch	2.15 KB	jojonaloha
#1	domain-use_left_joins-1422040-1.patch	2.15 KB	jojonaloha

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jojonaloha CreditAttribution: jojonaloha commented 31 January 2012 at 00:11

Status:

Active

» Needs review

File	Size
domain-use_left_joins-1422040-1.patch	2.15 KB

Here is the proposed patch.

Comment #2

agentrickard

he/him

English

Georgia (US)

CreditAttribution: agentrickard commented 31 January 2012 at 18:06

Status:

Needs review

» Needs work

Two issues:

1) The patch doesn't conform to coding standards.
2) How do you test this?

Comment #3

jojonaloha CreditAttribution: jojonaloha commented 21 February 2012 at 01:23

File	Size
domain-use_left_joins-1422040-3.patch	2.15 KB

1) I think the attached patch fixes this, I've read http://drupal.org/coding-standards and still had a hard time finding issues
2) I can describe my use-case a little which may help.
I have domain, domain_alias, domain_conf, domain_content, domain_settings and domain_theme enabled.

I have a few content types: We can call them "Event" and "Moderator"
A moderator is not a user but has a picture and a bio.
An event may have 1 moderator, using a nodereference field. It is an optional field.

I have a view that displays all Events for the current domain with the Moderator's picture, linked to their bio. I need the view to display all events even if there is no moderator. In my view I have a relationship to the Moderator which is not required.

This worked as I expected before the update to Views 7.x-3.1, after the update the query changed so that node_access is joined for Moderator as well. By using an INNER JOIN between node_access and the moderator, it was forcing the relationship to be required.

Comment #4

agentrickard

he/him

English

Georgia (US)

CreditAttribution: agentrickard commented 21 February 2012 at 13:38

Status:

Needs work

» Needs review

Looks good. I see the logic here. This is a problem caused only for administrators.

What is odd to me is that core doesn't seem to use this pattern. From _node_query_node_access_alter():

      // The node_access table has the access grants for any given node so JOIN
      // it to the table containing the nid which can be either the node
      // table or a field value table.
      if ($type == 'node') {
        $access_alias = $query->join('node_access', 'na', '%alias.nid = ' . $nalias . '.nid');
      }
      else {
        $access_alias = $query->leftJoin('node_access', 'na', '%alias.nid = ' . $nalias . '.entity_id');
        $base_alias = $nalias;
      }

Do you have a View you can export and attach for testing?

Comment #5

jojonaloha CreditAttribution: jojonaloha commented 21 February 2012 at 16:39

File	Size
domain-use_left_joins-1422040-example_view.txt	26.19 KB

Here is my exported view, I stripped out some of the extra displays and client info.

Comment #6

agentrickard

he/him

English

Georgia (US)

CreditAttribution: agentrickard commented 8 March 2012 at 22:26

This is going to have to wait for the dust to clear on #681760: Try to improve performance and eliminate duplicates caused by node_access table joins.

Comment #7

agentrickard

he/him

English

Georgia (US)

CreditAttribution: agentrickard commented 11 March 2012 at 19:30

Status:

Needs review

» Postponed

Comment #8

jojonaloha CreditAttribution: jojonaloha commented 16 April 2012 at 06:14

Version:	7.x-2.x-dev	» 7.x-3.x-dev
Status:	Postponed	» Needs review

File	Size
domain-node_access_joins-1422040-8.patch	3.08 KB

Noticed there was a fix committed for that issue in Drupal 7, http://drupal.org/node/681760#comment-5701576

I've attached a patch based on the accepted patch from that issue. I'm not sure it solves this issue, but at least its a starting point based on what is (will be) in core.