Local database location support for fields with multiple entries

Thanks for reporting this problem!
Unfortunately, it seems I merged those changes a bit prematurely, there are several parts about this functionality that are not working quite as they should. ( 🐛 Performance degradation after upgrading to 8.x-1.29 Fixed would be another example.)

The problem here, it seems, is that location conditions are always placed on the index base table, with only the single-valued fields, so only the first field value is taken into account (I’d guess). Instead, if the field is multi-valued, we’d need to use the field-specific table.
However, there are also other bugs I immediately spotted in the code, and some of the code I don’t quite understand, so there are also other potential sources of this problem. I’ll try to find the time to take a closer look at this at some point, but the truth is that I’m pretty overworked already with contrib work, and this is a rather niche functionality that would probably take a significant amount of time to fix. Therefore, it might take a while until I get to it.

If you are able to provide a patch for this yourself (given my observations above) that resolve the problem for you, this would help a lot. I’d then be happy to review and help you get it over the finish line. It would need automated test coverage, though, in any case.

https://git.drupalcode.org/issue/search_api-3353060/-/tree/3353060-db-lo... passes tests locally, and a very cursory test by usage. So for sure requires more testing and eyes on it. It does change the LocationAwareDatabaseInterface.

Thanks a lot, sounds great!
However, since testing for issue forks still doesn’t work in this project (cf. #3190024: Problem with test dependencies when testing issue forks → ), could you please post a patch with your changes? Thanks!

Thanks!
I cleaned up the code a bit and also added the regression test.
However, the big problem remains that we cannot really change an existing interface. Not quite sure how to get around that – normally, we have a deprecation as an intermediate step, but it’s unclear to me how that would look in this instance.
Maybe put the new parameters at the end of the parameter list and comment them out? But then how do we check if the implementing class has been updated – reflection?

I’m also not sure why you’re not calling LocationAwareDatabaseInterface::addLocationFilter() anymore from within Database. Is that really necessary, couldn’t this also work some other way? The contract of that method states it will always be called first, so this change would actually somewhat violate the interface contract.

I think I’ll have to take another look when I have more time.

Thanks for the work on this patch.
This is working for when there are multiple locations.
I've discovered there is an issue when adding facets to a search api view with location, where the query looks like its adding the location query multiple times and is preventing the query working with this patch.
I get this error: A database exception occurred while searching.
and then no search results.

It looks like the method createDbQuery in modules/search_api_db/src/Plugin/search_api/backend/Database.php is where the locations fields are addded
$this->addLocationConditions($condition_group, $query);
But since this gets called not just when assembling the original query but also when facets are being built in the getFacets method

I've managed to resolve the issue I encountered with the facets by cloning the $query in createDbQuery with the passed in $query, patch attached.

@andybroomfield: Thanks for that!

However, when creating patches based on existing ones, please remember to include an interdiff → in the future. This makes them easier to review. (Though this will soon become moot, when we switch to working with issue forks and MRs.)

Also, could you maybe provide a failing test case for your problem with the patch in #9? That would help a lot in making sure this keeps working.

NW for the tests, and also because I’m still unsure about the correct way to implement the fix to avoid problems with backwards compatibility (see #9).