Problem/Motivation

Search API queries containing both AND'd facets and a search_api_location option will cause a DB error.

Steps to reproduce

Create index to with a location (lat/lon) field, also add a field for a facet to be configured as AND. Complete a search including search_api_location option (for example using the search_api_location_views module - but this is completely not required - I discovered the original adding the location to the query myself).

Outline of reason for error

When retrieving the AND (not OR) facets a temporary table is made https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/modules/sea...
The method to do this removes all fields except item_id and unsets the expressions https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/modules/sea... it does not remove conditions, or having, leaving a $db_query at https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/modules/sea... of

"SELECT DISTINCT "t"."item_id" AS "item_id"
FROM
{search_api_db_locations} "t"
HAVING "location__distance" < :db_condition_placeholder_0"

Unsetting the expression means that the location__distance field defined in the having isn't in the query. Exception. Leaving the expression intact causes the score to be added in addition to the location__distance field.

"SELECT DISTINCT "t"."item_id" AS "item_id", :score AS "score", ST_Distance_Sphere(Point(:centre_lon, :centre_lat), ST_PointFromText(t.location)) / 1000 AS "location__distance"
FROM
{search_api_db_locations} "t"
HAVING "location__distance" < :db_condition_placeholder_0"

Proposed resolution

Question: What is the required for the temporary table?

Does it include the conditions (these haven't been unset), in which case it should include the having. If this is the case, but other expressions must be removed (for performance reasons? I'm guessing I'd need to understand exactly how the AND facet queries are constructed), then edge casing the location expression seems like the only option.

If the temporary table could include the expressions then just removing the lines that make them an empty array round line 2893 is simpler.

If there aren't supposed to be conditions, unlikely, the having could be removed.

CommentFileSizeAuthor
#6 3392465-6--db_backend_location_filter_plus_and_facet.patch4.63 KBdrunken monkey
#6 3392465-6--db_backend_location_filter_plus_and_facet--tests_only.patch3.31 KBdrunken monkey

Issue fork search_api-3392465

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ekes created an issue. See original summary.

ekes’s picture

ekes commented

I can add:

I did try and write a (failing) test for this. There don't seem to be any tests for AND facets in search_api/tests at least, only OR? When I tried adding an AND facet in a Kernel test something like

    // Search with a AND facet.
    $location_options = [
      [
        'field' => 'location',
        'lat' => '51.260197',
        'lon' => '4.402771',
        'radius' => '500',
      ],
    ];
    $query = $this->buildSearch(place_id_sort: FALSE)->sort('location__distance');
    $query->setOption('search_api_location', $location_options);
    $conditions = $query->createAndAddConditionGroup('AND', ['facet:' . 'category']);
    $conditions->addCondition('category', 'item_category');
    $facets['category'] = [
      'field' => 'category',
      'limit' => 0,
      'min_count' => 1,
      'missing' => TRUE,
      'operator' => 'and',
    ];
    $query->setOption('search_api_facets', $facets);
    $results = $query->execute();

I ended up with a PHPUnit\Framework\Exception: PHP Fatal error: Uncaught AssertionError: The container was serialized. in /var/www/html/web/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php:88

Finn Lewis made their first commit to this issue’s fork.

finn lewis’s picture

finn lewis
Location Oxford
commented

I can confirm the behaviour on my local dev environment.

I've created an issue fork and a commit that I can use as a patch for now to test hacking those two lines out of search_api_db.

https://git.drupalcode.org/issue/search_api-3392465/-/commit/de1cc3c5d4a...

I really don't understand what is happening here, but looking back in time, those lines come from the Drupal 7 version of search_api_db 10 years ago, possibly around removing the score?

https://git.drupalcode.org/project/search_api_db/-/blame/7.x-1.x/service...

https://git.drupalcode.org/project/search_api_db/-/blob/7.x-1.0-rc1/serv...

https://git.drupalcode.org/project/search_api_db/-/commit/5aee9c0c63a08c...

ekes’s picture

ekes commented

The logic has always been there since facet support was first introduced https://git.drupalcode.org/project/search_api_db/-/commit/275c74c6b6ebb4... although there it seems even clearer the purpose is to keep conditions and even sorts (which would in postgres later add another field iirc), so it's just removing unnecessary fields (like score... are there others?)

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 3392465-6--db_backend_location_filter_plus_and_facet--tests_only.patch3.31 KB
new 3392465-6--db_backend_location_filter_plus_and_facet.patch4.63 KB

Thanks for reporting this problem!
I could easily reproduce it, the attached patch should fix it.
I also had no problem with writing a test, not sure what went wrong for you. You are right, in any case, that it is pretty weird that we don’t include any test for AND facets, so I also fixed that (even though pretty out of scope, of course – sorry!).

ekes’s picture

ekes commented

Explicitly unsetting score makes sense.

  • drunken monkey committed 4c20bcc0 on 8.x-1.x 
    Issue #3392465 by drunken monkey: Fixed use of location filter together...
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

Great, thanks for reporting back!
Merged. Thanks again!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.