Problem/Motivation

Steps to reproduce the problem:
- create a content type using "list" fields: List (integer), List (float), List (text), and Boolean
- create a view using the content type as base table
- add two filters on the same list field using the "Is one of" operator
- separate the filters into two groups
- connect the filter groups by "OR"; connect the filters in each group by "AND"
- optionally, expose the filters

Example:
group 1 (AND each filter)
- content type = type1
- published = true
- field_1 = value1A
- field_2 = value2A (exposed)
- field_3 = value3A (exposed)
OR
group 2 (AND each filter)
- content type = type1
- published = true
- field_1 = value1B
- field_2 = value2B (exposed)
- field_3 = value3B (exposed)

The join clauses for field_1 are:
INNER JOIN {field_data_field_1} field_data_field_1 ON
node.nid = field_data_field_1.entity_id AND
(field_data_field_1.entity_type = 'node' AND field_data_field_1.deleted = '0')

LEFT JOIN {field_data_field_2} field_data_field_2 ON
node.nid = field_data_field_2.entity_id AND
field_data_field_2.field_2_value != '1'

In the example, the second join clause is unnecessary and its presence causes the results to exclude desired values. It occurs because the code does not respect the filter groups when determining the join clause.

Proposed resolution

Respect the filter groups when determining the join clause for this type of filter.

Class hierarchy for a "list" module field:

views_handler
views_handler_filter
views_handler_filter_in_operator
views_handler_filter_many_to_one
views_handler_filter_field_list

The offending code is in three methods of views_many_to_one_helper class: ensure_my_table(), summary_join(), and add_table.

Remaining tasks

Review, write tests.

User interface changes

None.

API changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

solotandem’s picture

Status: Active » Needs review
FileSize
4 KB

The attached patch implements group checking for the many-to-one filter handler. Similar code appears in the many-to-one argument handler, but I did not test this and it will await a later patch.

mradcliffe’s picture

I confirmed this, and the patch still applies to 7.x-3.x and works for me.

Chris Matthews’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

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

Checking patch includes/handlers.inc...
Hunk #1 succeeded at 897 (offset 63 lines).
Hunk #2 succeeded at 929 (offset 68 lines).
error: while searching for:
      // We hence get the absolute simplest:
      $field = $this->handler->relationship . '_' . $this->handler->table . '.' . $this->handler->field;
      if ($this->handler->operator == 'or' && empty($this->handler->options['reduce_duplicates'])) {
        if (empty($this->handler->options['add_table']) && empty($this->handler->view->many_to_one_tables[$field])) {
          // query optimization, INNER joins are slightly faster, so use them
          // when we know we can.
          $join = $this->get_join();

error: patch failed: includes/handlers.inc:895
error: includes/handlers.inc: patch does not apply
mradcliffe’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.63 KB
2.77 KB

I've attached a re-roll. The patches are too old for interdiff to work so I've attached a manual diff of patches.

I'm still using the patch in #1.

Chris Matthews’s picture

Thanks @mradcliffe, the re-rolled patch #4 to handlers.inc applied cleanly to the latest views 7.x-3.x-dev and fixes this issue for me, but others may want to weigh in as well so I'll leave the status as is for now.