Filter "not" is not working

Thanks for reporting this problem!
However, I fear I cannot reproduce this, and we also have automated tests in place to ensure this works correctly.

Do you maybe have the "Parse mode" for the "Fulltext search" filter set to "Direct"? In this case, please try switching to "Multiple words".
Also, what server backend are you using?

PS: It seems you (like many others – it's really easy to misinterpret) are confused by the "Issue tags" field. As the guidelines state, they aren't meant for free text tags related to the issue, but only for specific categorization purposes, usually by module maintainers.
So, if you aren't sure your current usage is correct, please just leave the field empty.

Marked two issues as duplicates, so this seems to be a more wide-spread problem than I thought. However, I still can‘t reproduce this, so I’ll need more information: used backend, entered keywords, minimum steps to reproduce (from clean installation) any displayed/logged errors, etc.

With the additional information in #3011084: Views Filter "Contains none of these words" doesn't work, I was finally able to reproduce this problem – it only occurs in conjunction with partial matching.
The attached patch should fix this, and contains test coverage to ensure this keeps working.

Thanks for the patch. This fix the error message i reported on this issue.

But unfortunately the search works not in all cases. E.g. when there are multiple search filters.

Here is what works:

• search for: not contain "john"
• search for: "lisa" and not "john"

When I have multiple words connected with "and" or "or" then the search result is always empty:

• search for: "lisa" and "max" and not "john"
• search for: "lisa" or "max" and not "john"

Here is the Query from the views preview:

Index: objects
Keys: 'john'
Parsed keys: array (
    '#conjunction' => 'AND',
    0 => 
    array (
      0 => 'lisa',
      1 => 'max',
      '#conjunction' => 'AND',
    ),
    1 => 
    array (
      0 => 'john',
      '#conjunction' => 'OR',
      '#negation' => true,
    ),
  )
Searched fields: fields...
Sorting: field_dating ASC
Options: array (
    'search_api_view' => 'object (Drupal\\views\\ViewExecutable)',
    'search_api_base_path' => 'suche',
  )

@drunken monkey can you reproduce the problem I described above?
For Feedback, I would be very grateful.

Sorry for the delay, I had a very busy December.
Yes, I could indeed reproduce the problem! After a bit of debugging, I luckily also found its cause: a re-used alias led to a wrong substitution resulting in unfulfillable conditions.
The attached revision for my above patch should resolve this problem, too. Please test/review!

1. +++ b/modules/search_api_db/src/Plugin/search_api/backend/Database.php
   @@ -2067,13 +2076,29 @@ protected function createKeysQuery($keys, array $fields, array $all_fields, Inde
          if ($conj == 'AND') {
            foreach ($negated as $k) {
   -          $db_query->condition('t.item_id', $this->createKeysQuery($k, $fields, $all_fields, $index), 'NOT IN');
   +          $nested_query = $this->createKeysQuery($k, $fields, $all_fields, $index);
   +          // For a "NOT IN", the SELECT must not have more than one column.
   +          $num_fields = count($nested_query->getFields());
   +          $num_expressions = count($nested_query->getExpressions());
   +          if ($num_fields + $num_expressions > 1) {
   +            $nested_query = $this->database->select($nested_query, 't')
   +              ->fields('t', ['item_id']);
   +          }
   +          $db_query->condition('t.item_id', $nested_query, 'NOT IN');
            }
          }
          else {
            $or = new Condition('OR');
            foreach ($negated as $k) {
   -          $or->condition('t.item_id', $this->createKeysQuery($k, $fields, $all_fields, $index), 'NOT IN');
   +          $nested_query = $this->createKeysQuery($k, $fields, $all_fields, $index);
   +          // For a "NOT IN", the SELECT must not have more than one column.
   +          $num_fields = count($nested_query->getFields());
   +          $num_expressions = count($nested_query->getExpressions());
   +          if ($num_fields + $num_expressions > 1) {
   +            $nested_query = $this->database->select($nested_query, 't')
   +              ->fields('t', ['item_id']);
   +          }
   +          $or->condition('t.item_id', $nested_query, 'NOT IN');
            }
            if (isset($old_query)) {
              $or->condition('t.item_id', $old_query, 'NOT IN');
   

   This introduces unneeded code duplication. I propose to move the nested query generation to before the if statement like so:

         $nested_queries = [];
         foreach ($negated as $k) {
           $nested_query = $this->createKeysQuery($k, $fields, $all_fields, $index);
           // For a "NOT IN", the SELECT must not have more than one column.
           $num_fields = count($nested_query->getFields());
           $num_expressions = count($nested_query->getExpressions());
           if ($num_fields + $num_expressions > 1) {
             $nested_query = $this->database->select($nested_query, 't')
               ->fields('t', ['item_id']);
           }
           $nested_queries[] = $nested_query;
         }
   
         if ($conj == 'AND') {
           foreach ($nested_queries as $nested_query) {
             $db_query->condition('t.item_id', $nested_query, 'NOT IN');
           }
         }
         else {
           $or = new Condition('OR');
           foreach ($nested_queries as $nested_query) {
             $or->condition('t.item_id', $nested_query, 'NOT IN');
           }
           if (isset($old_query)) {
             $or->condition('t.item_id', $old_query, 'NOT IN');
           }
           $db_query->condition($or);
         }

2. +++ b/modules/search_api_db/tests/src/Kernel/BackendTest.php
   @@ -687,6 +693,66 @@ protected function regressionTest2994022() {
   +      $data_sets = [
   +        'not this word' => [
   +          'keys' => [
   +            '#conjunction' => 'OR',
   +            '#negation' => TRUE,
   +            'test',
   +          ],
   +          'results' => [1, 3, 4, 5],
   +        ],
   +        'none of these words' => [
   +          'keys' => [
   +            '#conjunction' => 'OR',
   +            '#negation' => TRUE,
   +            'test',
   +            'foo',
   +          ],
   +          'results' => [3, 5],
   +        ],
   +        'not all of these words' => [
   +          'keys' => [
   +            '#conjunction' => 'AND',
   +            '#negation' => TRUE,
   +            'foo baz',
   +          ],
   +          'results' => [2, 3, 5],
   +        ],
   +        'complex keywords' => [
   +          'keys' => [
   +            [
   +              'foo',
   +              'bar',
   +              '#conjunction' => 'AND',
   +            ],
   +            [
   +              'test',
   +              '#conjunction' => 'OR',
   +              '#negation' => TRUE,
   +            ],
   +            '#conjunction' => 'AND',
   +          ],
   +          'results' => [1],
   +        ],
   +      ];
   

   This is basically a dataprovider, lets make it official and actually put it in a @dataProvider :)

About #9.2. we've tried to be as mindful as possible about using d.o resources (and thus keeping test runs as fast as possible). You'll see other places where we do something similar, as we want to make sure we do the minimal amount of drupal installs as possible during the test runs.

That's we do things like this in browser tests.

This is a kernel test, so they are fast enough as is. I think the added readability of a dataprovider is a win here.

Basically a long-winded way of saying +1 ;)

Thanks again for the patch. Now the search for multiple words connected with "and" and "and not" works (e.g. "lisa" and "max" and not "john").

But when I connect multiple words with "or" and "and not" (e.g. "lisa" or "max" and not "john") then the conjunction changes from "or" to "and".

@drunken monkey can you reproduce this behavior?

@ legolasbo: Thanks a lot for the thorough review!
I agree with your first note, that could really be written better. I tried it in the attached revision, please tell me what you think.

Regarding the second note: I generally agree, data providers are superior to just having the data sets inline, like I did in #8.
However, in this case, the problem is the nested loop, over both data sets and match mode. Converting this to a data provider would result in twelve data sets, with a lot of duplication, which not only subtracts a lot from the readability, but also takes considerable time to run: a bit over two minutes in my case (whereas the whole testBackend() just needs 22 seconds). Kernel tests are still not as cheap as Unit tests, and the whole setup does take its time.
Finally we currently have all regression tests as sub-methods of the main testBackend() test method, so the proposed code in #8 is also more consistent with existing tests. (Though that’s the least important argument here.)

I still changed the implementation to a data provider in the attached patch, but would advise against actually committing it in this state.

But when I connect multiple words with "or" and "and not" (e.g. "lisa" or "max" and not "john") then the conjunction changes from "or" to "and".

Do you set that with multiple “Search: Fulltext search” filters in the view, or with custom code?
Can you use the live preview’s “Show query” functionality to get the Search API query being executed for that view?
In general, combining multiple “Search: Fulltext search” filters doesn’t really work as it should, due to technical restrictions of the Search API architecture.

Hmm, I've run the tests locally, both before, with #8 and with #12 and see your point about the time it takes to run these tests. I think we've got ourselves a bit of a conundrum here, where on one hand we can have optimum test accuracy and on the other hand have optimum test execution speed.

After having thought about it some more and a thorough look at how the tests operate I've come to the conclusion that the test actually tests 4 searches in 3 different configurations. The searches themselves don't change anything about the test setup and can therefore safely be preformed in the same iteration to safe resources. I therefore propose to have a simple dataprovider that only provides the match mode and then run the searches themselves within each iteration to reduce the number of test setups from 12 to 3 without the risk of the tests contamination each other.

Your other changes are fine by me :)

@drunken monkey
Yes I do have multiple “Search: Fulltext search” filters in the view. I have three “Search: Fulltext search” filters. One for the operator "Contains all of these words", one for "Contains any of these words" and one for "Contains none of these words".

Here is the Query from the views preview for the search "lisa" or "max" and not "john" where I had inserted "lisa max" in the filter field "Contains any of these words" and "john" in the filter field "Contains none of these words".

Index: objects
Keys: 'john'
Parsed keys: array (
    '#conjunction' => 'AND',
    0 => 
    array (
      0 => 'lisa',
      1 => 'max',
      '#conjunction' => 'AND',
    ),
    1 => 
    array (
      0 => 'john',
      '#conjunction' => 'OR',
      '#negation' => true,
    ),
  )
Searched fields: fields ...
Sorting: field_dating ASC
Options: array (
    'search_api_view' => 'object (Drupal\\views\\ViewExecutable)',
    'search_api_base_path' => 'suche',
  )

Is there any chance to fix the problem from comment #11?

But when I connect multiple words with "or" and "and not" (e.g. "lisa" or "max" and not "john") then the conjunction changes from "or" to "and".

@ Keule: Ah, I see the problem now!
Please see #3031991: Fix edge case errors when combining multiple fulltext search filters in Views.

@ Legolasbo: Hm, really not sure. Half a minute of runtime just for a single issue’s regression testing still seems a lot to me. After all, this will run for every single patch posted for this module (in D8).
I also have to admit I generally don’t understand the “test accuracy” argument much. This will almost never fail, and when it does, it should still be easy to see why – or you can modify the test a bit at that point to see it more clearly. But doing all of this beforehand for a scenario that might never arise seems a lot like a variant of “premature optimization” to me.

@ Joris: What do you say, are you +1 on the compromise in #13 or do you also find half a minute too much?

In case we stick with this, the $matchMode should be changed back to snail_case, so this needs work regardless.

Half a minute is a lot, but I think that the tests are more readable with the compromise in #13. I agree with the change back to snake(snail) case.

Alright, then I’m washing my hands off this and will just direct the Test Bot Resources Police to your doors when they come knocking … ;P

Should be RTBC now?

Yes!

Alright, thanks!
Committed.
Thanks again, everyone!