Add support for OR facets

I've tried this patch and the dependency and found that they both apply cleanly, however I'm running in to an issue which might just be my understanding but perhaps not.

Currently I'm trying this on a View (of commerce products) with 3 facets (color, manufacturer, type), each of which is set up as an 'or' thanks to this patch. The goal was a "free form" list of products which should never return an empty set. For example, if I select the colours "red" and "blue", ideally the manufacturers facet options should be reduced to the manufacturers that offer red and blue products (and similarly for type). If I then select "manufacturer A" and "manufacturer B" the type facet options should be reduced to types of products which are offered by "manufacturer A" or "manufacturer B" and are "red" or "blue".

The behaviour I'm seeing at the moment is if I select the colours "red" and "blue", there are type options that aren't removed from the facet options, even though there are no "red" or "blue" products with that type, thus it's possible to get to an empty result.

If this doesn't make sense please let me know I'll try and get in touch with you on IRC with an example site. I'm setting this to needs work but if my understanding of how this should work is wrong please set it back and I'll do my best to give it a proper review. Thanks!

Just saw http://drupal.org/node/1204966#comment-5417670 which I guess could be related?

Marked #1424690: Support for OR (inclusive) facets as dupe of this one.

is this supposed to be working with FacetAPI?
I get "operator not defined" error all the time, since there is no $facet['operator'] at all.

okay, I see it uses FacetAPI constants :) how can I define "operator" element in facet array?

I'll postpone this now, until we commit #1390598: Add a way to easily identify facet filters inside the query.
I'll also take a closer look at the patch then, I've now just skimmed over it.

Thanks, in any case!

Bug in #2 patch.

Need handle clone somehow. Now this code, doesn't clone filters of the object.

$or_query = clone $query;

As result we looses filters in $query after unset it in $or_query

unset($filters[$filter_id]);

Fast patch. Extended class SearchApiQuery with __clone() function.

Also $fields initialization missed after patch. This part was moved to function buildDatabaseQuery :

        $index = $query->getIndex();
        if (empty($this->options['indexes'][$index->machine_name])) {
            throw new SearchApiException(t('Unknown index @id.', array('@id' => $index->machine_name)));
        }
        $fields = $this->options['indexes'][$index->machine_name];

But due the fact that this array used in next part of code, we need initialize $fields in function search() as well.

                    if (!isset($fields[$field_name])) {
                        throw new SearchApiException(t('Trying to sort on unknown field @field.', array('@field' => $field_name)));
                    }

Hello,
Is there any news regarding this issue? bit confused from the patch parts mentioned above.
If there is a patch file that worked for you, I'll be happy to test it.
Thanks, Yael

Echoing yaelsho... any news on this one? Is "OR" supported now for DB search?

Same question as the previous askers - any updates on this issue? Thanks.

Sorry for leaving this lying here for so long, it was high up on my TODO list, I've just got very little time for such larger feature requests. But I'm sure we can now get this committed in the coming weeks!

I've moved the cloning problem to a new issue (since it has to be handled in the Search API itself anyways) and provided a complete patch there: #2144531: Fix cloning of queries to clone filters, too.

Regarding the patch by Damien itself, that already works and looks brilliantly, thanks a lot again! It of course needed a re-roll, but other than that there was only one tiny mistake (the operator key for facets is optional). I also added some tests, and I think we should now be all set.
Please test/review!

Ah. Yes. Of course. We'll have to wait (at least) for Search API 1.10 to be released before we can commit this. We maybe should add a versioned dependency there, too. On the other hand, the error is only triggered when using OR facets, so maybe documenting this is enough.

So, does anyone want to test or review this?
(Will let the test bot test in a few days.)

Here is my implementation of Search API DB module OR logic (inside 1 facet).
It's based on posted patches adapted to last module version + additional logic for handling "No results" case and display all facets (have ability unselect it).
Patch required current latest Search API (7.x-1.10) and Search API DB (7.x-1.1) modules version.
Sponsored by Skilld

Fixed path to patched file.

Using facet operator exceptions fixed.

Patch format fixed.

1. +++ b/search_api_db.test
   @@ -45,6 +45,7 @@ class SearchApiDbTest extends DrupalWebTestCase {
   +    $this->checkFacets();
   
   @@ -237,6 +238,56 @@ class SearchApiDbTest extends DrupalWebTestCase {
   +  protected function checkFacets() {
   +    $query = $this->buildSearch();
   

   Anton, you forget to add test for patch

2. +++ b/service.inc
   @@ -1482,7 +1483,24 @@ class SearchApiDbService extends SearchApiAbstractService {
   +      if (empty($facet['operator']) || $facet['operator'] != 'or') {
   +        // All the AND facets can use the main query.
   

   do we really need to skip throwing excepion in case od unknown operator?

3. +++ b/service.inc
   @@ -1482,7 +1483,24 @@ class SearchApiDbService extends SearchApiAbstractService {
   +        $tag = 'facet:' . $facet['field'];
   ...
   +          if ($filter instanceof SearchApiQueryFilterInterface && $filter->hasTag($tag)) {
   +            unset($filters[$filter_id]);
   

   do tags enough to unset?

Anton's patch review

1. +++ b/service.inc
   @@ -992,6 +993,11 @@ class SearchApiDbService extends SearchApiAbstractService {
   +      // Display facets with selected items if no refults found.
   +      if ($query->getOption('search_api_facets')) {
   +        $query->noResultsFound = TRUE;
   +        $results['search_api_facets'] = $this->getFacets($query, clone $db_query);
   

   is this change related?

2. +++ b/service.inc
   @@ -1560,7 +1566,52 @@ class SearchApiDbService extends SearchApiAbstractService {
   +                // If no results found - remove all filters from facet query
   +                // for "see" all selected facets and has ability "uncheck" it.
   +                if (isset($query->noResultsFound)) {
   +                  if (isset($filters[$filter_id])) {
   +                    unset($filters[$filter_id]);
   

   this could be moved out of internal loop

3. +++ b/service.inc
   @@ -1560,7 +1566,52 @@ class SearchApiDbService extends SearchApiAbstractService {
   +                  if (in_array($facet['field'], $filter_items)) {
   +                    unset($filters[$filter_id]);
   

   loops is not needed here

1.1 Anton, you forget to add test for patch
-> Patch was updated and now it is combination of previous version + 1390586-15--or_facets.patch

2.1 is this change related?
-> This change related because it handle "No results" case and help "end user": Give ability uncheck incorrect facet items and not lose already selected filters. This part was reworked on current patch.

2.2 this could be moved out of internal loop
-> Fixed.

2.3 loops is not needed here
-> Fixed.

Fixed test error.

Fixed path to patched files.

And let's get rid of drupal_map_assoc() as well

@ Antonnavi, andypost: Thanks a lot for your work on this!
However, Antonnavi's additional change is definitely not in the scope of this issue! Please create a new one for that, elaborate on your reasoning and amend the patch with a server option to turn this behavior on and off.

As all other changes were, I think, only related to the new code, a re-roll of my patch in #15 should be OK, right? (I actually found a bug in one of the new tests, maybe that was the cause of the previous fails.)
Please find it attached and see if it works for you, or you find anything wrong with it. Then I can hopefully finally commit this.

Last added patch #39 works fine for me.
New issue No facets on no results was created (with detailed description and patch for fix it).

OK, great to hear!
Committed.
Thanks a lot again to everyone who worked on this – especially, of course, to Damien!