We're using search API in the WissKI module context and load data from a triple store. For historical reasons and due to translatability/revisionablity of content we do a hybrid approach - current data is stored in the triple store and everything that does not fit there (like revisions) is stored in sql. However for this approach we need to have a basetable (which is typically empty). This breaks with the current version of search api because the tracking does not give any results anymore due to:
search_api/src/Plugin/search_api/datasource/ContentEntity.php line 801:
// Use a direct database query when an entity has a defined base table. This
// should prevent performance issues associated with the use of entity query
// on large data sets. This allows for better control over what tables are
// included in the query.
// If no base table is present, then perform an entity query instead.
if ($entity_type->getBaseTable()) {
$select = $this->getDatabaseConnection()
->select($entity_type->getBaseTable(), 'base_table')
->fields('base_table', [$entity_id]);
}Install WissKI and do some tracking...
Please make this switchable in the configuration for search api - accessing the entity tables directly is a bad practice and assuming it is feasible due to the existence of a basetable is a very implicit assumption! I guess many more modules will have problems with that and assuming the entities storage typically is SQL just makes it even worse...
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3247840-2--option_to_disable_db_query_for_tracking.patch | 1.78 KB | drunken monkey |
Comments
Comment #2
drunken monkeyThanks for reporting this problem!
We were of course aware that this assumption might not be true in all cases, but it’s hard to determine beforehand whether anyone will actually be affected, or how many people/sites will, as only very few people participate in the issue queue and help by testing patches.
As yours has been the only bug report so far, with just you as a follower, it seems there aren’t many people affected. Therefore, I think having a hidden option to disable this behavior should be fine. We can always add a UI for it later, in case it turns out this affects more people after all.
Please test/review the attached patch, which adds a
disable_db_trackingsetting for datasources. You’ll have to manually set it in the configuration – e.g., like this:Or by adding
disable_db_tracking: trueto thedatasource_settings.'entity:FOO'entry of your search index’s YAML and then re-importing.When testing, please also make sure that editing the index doesn’t remove/reset that option.
Additionally, do you know of a clean way to spot your specific problem? I’d be open to adding a specific check for that, if it’s simple enough. (E.g., if only a specific entity type is affected.)
Comment #3
drunken monkeyWould be great to get feedback on this before committing.
Comment #4
knurg commentedYes, works great! Thank you :)
However I would suggest du fix some lines below in the same way:
if ($page > 0) {
// We only handle the case of picking up from where the last page left
// off. (This will cause an infinite loop if anyone ever wants to index
// Search API tasks in an index, so check for that to be on the safe
// side.)
if (isset($last_ids[$context_key])
&& $last_ids[$context_key]['page'] == ($page - 1)
&& $this->getEntityTypeId() !== 'search_api_task') {
$select->condition($entity_id, $last_ids[$context_key]['last_id'], '>');
$offset = 0;
}
}
Should probably be:
if ($page > 0) {
// We only handle the case of picking up from where the last page left
// off. (This will cause an infinite loop if anyone ever wants to index
// Search API tasks in an index, so check for that to be on the safe
// side.)
if (isset($last_ids[$context_key])
&& $last_ids[$context_key]['page'] == ($page - 1)
&& $this->getEntityTypeId() !== 'search_api_task'
&& empty($this->configuration['disable_db_tracking']) ) {
$select->condition($entity_id, $last_ids[$context_key]['last_id'], '>');
$offset = 0;
}
}
Comment #6
drunken monkeyGood to hear, thanks for testing!
When posting code, though, please put
<?php … ?>tags around it, as it otherwise becomes pretty much unreadable.Also, I’m pretty sure the code you reference should still run in either case, so the patch seems fine as it is. Or how would the
disable_db_trackingoption relate to this?Anyways, I committed the patch now in this form. (If you convince me the lines below should still be changed, we can still do that later.)
Thanks again!