Changes in search query functionality

Overall this looks great and I think this is almost ready, I found a couple small nitpicks though

1. +++ b/search_api.api.php
   @@ -147,16 +147,44 @@ function hook_search_api_items_indexed(\Drupal\search_api\IndexInterface $index,
   +  // Otherwise, exclude all nodes with NID 10.
   

   I would rephrase this comment: "Exclude the node with ID 10 from being indexed."

2. +++ b/search_api.api.php
   @@ -147,16 +147,44 @@ function hook_search_api_items_indexed(\Drupal\search_api\IndexInterface $index,
   +  // Do not run for queries with a certain tag.
   +  if ($query->hasTag('example_tag')) {
   +    return;
   +  }
   

   This example code is confusing, because we're already in the example code for a _TAG_ query alter, I'd remove these lines.

3. +++ b/search_api.api.php
   @@ -147,16 +147,44 @@ function hook_search_api_items_indexed(\Drupal\search_api\IndexInterface $index,
   +  // Otherwise, exclude all nodes with NID 10.
   

   Can we also use the comment suggested in .1 here?

4. +++ b/search_api.api.php
   @@ -147,16 +147,44 @@ function hook_search_api_items_indexed(\Drupal\search_api\IndexInterface $index,
   +  foreach ($query->getIndex()
   +    ->getDatasources() as $datasource_id => $datasource) {
   

   The indenting here is different compared to hook_search_api_query_alter, we should use the same style in both functions.

5. +++ b/search_api_db/src/Plugin/search_api/backend/Database.php
   @@ -1808,8 +1808,8 @@ class Database extends BackendPluginBase {
   +    $cond =  new Condition($conditions->getConjunction());
   

   Can we refactor this $cond variable to be $condition?
   I'd prefer the added readability over the saved bytes.

6. +++ b/search_api_db/src/Plugin/search_api/backend/Database.php
   @@ -1818,9 +1818,9 @@ class Database extends BackendPluginBase {
   +        $c = $this->createDbCondition($condition, $fields, $db_query, $index);
            if ($c) {
   

   Can we refactor this $c variable to be $condition?
   I'd prefer the added readability over the saved bytes.

7. +++ b/search_api_db/src/Plugin/search_api/backend/Database.php
   @@ -1828,33 +1828,37 @@ class Database extends BackendPluginBase {
   -        $field = $fields[$f[0]];
   +        $field_info = $fields[$field];
   

   This is a lot more readable, awesome!

8. +++ b/search_api_db/src/Plugin/search_api/backend/Database.php
   @@ -1828,33 +1828,37 @@ class Database extends BackendPluginBase {
   +        if ($value === NULL) {
   

   Should we use is_null instead?

9. +++ b/search_api_db/src/Plugin/search_api/backend/Database.php
   @@ -1931,6 +1935,9 @@ class Database extends BackendPluginBase {
   +    // @todo This is not actually necessary for OR facets. So, only do this if
   +    //   there are actually AND facets, and do not return right away if this
   +    //   fails.
   

   Can we create a new issue and link to that issue from the code or should we fix this in this issue?

10. +++ b/search_api_db/src/Tests/BackendTest.php
    @@ -203,7 +203,7 @@ class BackendTest extends EntityUnitTestBase {
        *   (optional) Filters to set on the query, in the format "field,value".
    

    Should we change this comment as well?

11. +++ b/src/Plugin/views/argument/SearchApiDate.php
    @@ -41,12 +41,12 @@ class SearchApiDate extends SearchApiArgument {
    +      //   necessary. $value_conditions will currently only ever contain a single
           //   child – a condition or a nested filter with two conditions.
    

    80 cols.

12. +++ b/src/Processor/FieldsProcessorPluginBase.php
    @@ -214,33 +214,39 @@ abstract class FieldsProcessorPluginBase extends ProcessorPluginBase {
    +   * Preprocesses the query condition.
    

    should this be "conditions"?

13. +++ b/src/Query/ConditionGroupInterface.php
    @@ -0,0 +1,57 @@
    +  public function &getConditions();
    ...
    +  public function &getTags();
    

    Can we explain why this is by reference in the comment?

14. +++ b/src/Query/QueryInterface.php
    @@ -59,23 +59,41 @@ interface QueryInterface {
    +   * filters-only query.
    

    Is this still filters or should this be conditions?

15. +++ b/src/Query/QueryInterface.php
    @@ -333,4 +313,68 @@ interface QueryInterface {
    +  public function &getTags();
    

    Can we document why this is by reference?

16. +++ b/tests/src/Unit/Plugin/Processor/FieldsProcessorPluginBaseTest.php
    @@ -302,18 +303,18 @@ class FieldsProcessorPluginBaseTest extends UnitTestCase {
       public function testProcessFilters() {
    

    Should we change this to testProcessConditions?

17. +++ b/tests/src/Unit/Plugin/Processor/FieldsProcessorPluginBaseTest.php
    @@ -302,18 +303,18 @@ class FieldsProcessorPluginBaseTest extends UnitTestCase {
    +      new Condition('text_field', '*foo'),
    

    This looks a lot better, ++.

18. +++ b/tests/src/Unit/Plugin/Processor/FieldsProcessorPluginBaseTest.php
    @@ -302,18 +303,18 @@ class FieldsProcessorPluginBaseTest extends UnitTestCase {
    +    $this->assertEquals($expected, $query->getConditionGroup()->getConditions(), 'Filters were preprocessed correctly.');
    

    Should this message be "Conditions were preprocessed correctly"?

19. +++ b/tests/src/Unit/Plugin/Processor/FieldsProcessorPluginBaseTest.php
    @@ -321,20 +322,20 @@ class FieldsProcessorPluginBaseTest extends UnitTestCase {
       public function testProcessFiltersNestedFilter() {
    ...
    +    $this->assertEquals($expected, $query->getConditionGroup()->getConditions()[0]->getConditions(), 'Filters were preprocessed correctly.');
    
    @@ -349,17 +350,17 @@ class FieldsProcessorPluginBaseTest extends UnitTestCase {
    +    $this->assertEquals($expected, array_merge($query->getConditionGroup()->getConditions()), 'Filters were preprocessed correctly.');
    

    See previous comments.

Also, if #2625264: Add tests for the module's alter hooks lands first, this needs an extra test for firing the tag hook.

Awesome. Very happy with your answers and the changed code, I'd say this is RTBC.