Some backends like the Solr backend are able to directly set all search result values. This works for aggregated fields, too.
But as soon as you add second aggregated field, Search API's VIews integration breaks the search result by overwriting the values from one aggregated field with the values from the other one.
The issue is caused by the fact that Search API hardcoded sets the property path of an aggregated field to 'aggregated_field'. As long as the backend doesn't provide the data, the issue is hidden. But if it provides the data, this piece of code becomes active:
namespace Drupal\search_api\Plugin\views\query;
class SearchApiQuery extends QueryPluginBase {
...
/**
* Adds Search API result items to a view's result set.
*
* @param \Drupal\search_api\Item\ItemInterface[] $results
* The search results.
* @param \Drupal\views\ViewExecutable $view
* The executed view.
*/
protected function addResults(array $results, ViewExecutable $view) {
...
// Gather any properties from the search results.
foreach ($result->getFields(FALSE) as $field_id => $field) {
if ($field->getValues()) {
$path = $field->getCombinedPropertyPath();
$values[$path] = $field->getValues();
}
...
}
...
}
}
...
}
For the database backend $field->getValues() doesn't return anything, but if the solr backend is configured to retrieve results this code fetches the values from the backend's result. But for any aggregated field the $path will be the same: 'aggregated_field'. As a consequence we only get the result of the last aggregated field because $values[$path] gets overwritten every time.
As a quick fix I tried to change the property paths in the index config to 'aggregated_field_1' and 'aggregated_field_2'. But this immediately caused errors in facets.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2927748.patch | 1.16 KB | mkalkbrenner |
Comments
Comment #2
mkalkbrennerIt turned out that there's already a solution but the code has the wrong order.
Comment #3
mstiem commentedI just tested that patch and it worked like a charm.
Comment #4
drunken monkeyThanks for reporting this issue and already fixing it!
I'm just not sure why this was a problem in the first place. As you might recall, we already fixed exactly this problem back in #2911734: Passing processor-generated field values from search results to Views is broken. And, sure,
$values['aggregated_field']would get overwritten with the values from the latest aggregatd field – but since that value shouldn't be used by the Views fields afterwards (instead it uses$values['aggregated_field|FIELD_ID'], which already should have the correct value in all cases), it's unclear to me why that would be a problem.I guess the proposed change also makes sense and shouldn't break anything, but it still would be very good if you could find out why it even matters. Where is
$row->aggregated_fieldcurrently used directly?Comment #5
mkalkbrennerI remember. That patch fixed the issue for one processor generated field. We didn't try it with two fields in Vienna.
The patch I provided here fixes it for multiple processor generated fields.
I stopped debugging as I reached this obvious bug which will be solved by the patch I provided. I think this should be committed anyway.
I you think that there might be more issues deeper in the Views integration, it would be better if you debug it on your own, because I'm not yet familiar with that views related code.
Comment #7
drunken monkeyHm, OK, then let's just go with that and see if anyone complains.
Would have been good to get a test for this as well, but our current test coverage of the Views integration doesn't extend to such special cases anyways, so I wouldn't say it's a must.