views_handler_relationship_node_term_data extremely slow

Mh skitch is kind of broken for german ISP's, at least i had the problem many times already.

Anyway.

Regarding this code:

+      $this->query->add_where($group, "$alias.machine_name", $vocabularies);
+      if (empty($this->options['required'])) {
+        $this->query->set_where_group('OR', $group);
+        $this->query->add_where($group, "$alias.machine_name", NULL);
+      }
+    }

Shouldn't this be caught already by the join type? INNER vs LEFT.

I don't sufficiently understand D7 taxonomy to be able to review this.

Now that I better understand D7 taxonomy, there's no way this patch can work. The whole reason for the subselect is that the condition is two joins away. You can't add the join conditionally in the non-required case and still have it work.

The WHERE doesn't work because you can then end up getting terms that are outside the specified vocabularies because they end up looking like NULLs.

I'll spend a few minutes testing this to be sure, but I'm pretty confident this construct doesn't work. I spent quite a bit of time with David Strauss trying to figure out the query that will work in this case.

Now, with D7, if you only care about a single vocabulary, which is the majority case, we *can* add a relationship on the field itself, which *does* know about vocabulary and that will work. So I do want to go that route.

Okay, that relationship already exists. Yay!

I did some testing of this patch. It doesn't fail in the way I thought, but the IS NULL check simply doesn't work.

I tried fiddling with it a little, but I couldn't make it work.

Scenario:

Create a view to list titles of a particular node type.
Set up a relationship specifying vocabularies that that node type cannot have. Make it not required.

Pre-patch: You get a simple list of nodes.
Post-patch: You get no results.

Unassign myself because the patch is already reviewed

Here we have 3 different situations I think.

1) No vocabulary supplied.
2) One or more vocabularies supplied and it is required.
3) One or more vocabularies supplied and it is not required.

For the first, we can have a simple join. For the third, we need the sub-query because we don't want to select terms from other vocabularies. For the second, I think we can just use the inner join.

Patch attached.

And I think there's another bug here. Please see the lines 18 and 19 in the patch. The relationship need to join to the added index table. Otherwise multiple relationships will use a same index table.

Patch updated.

An "ensure_table" with a custom join can generate duplicate joins with other "ensure_table"s. So I moved the join conditions to "where".

I also took back "$join->adjusted = TRUE;". But I don't know why it's there :P.

Triggering the testbot.

Puh it would be good to have an issue summary here, as this is not that easy to understand.

Hello, is this issue still alive? does this patch work on current views version?

I would like to know that too. I'm stil having the same problem with version 3.7

The 7 year old patch in #9 to views_handler_relationship_node_term_data.inc does not apply to the latest views 7.x-3.x-dev and if still applicable needs reroll.

Patch rerolled.