In the Solr backend, it is rather easy to override details of the search process, by using either the hook_search_api_solr_query_alter() and hook_search_api_solr_search_results_alter() hooks or subclassing the service class and overriding the preQuery() and postQuery() methods. The advantage of these hooks/methods compared to Search API's normal query/results alter hooks is that they allow access to the internal, Solr-specific information while altering, thus adding a lot of additional capabilities.

While the possibilities are probably much more limited in the Database Search backend, I still think it's a good idea to introduce these here, too. (Only the results alter hook doesn't really make sense, since we can't really add any additional information. However, the postQuery() method still makes sense because otherwise service class-scoped overriding wouldn't be easily possible.)

Do you think this makes sense? Please comment!

Estimated Value and Story Points

This issue was identified as a Beta Blocker for Drupal 8. We sat down and figured out the value proposition and amount of work (story points) for this issue.

Value and Story points are in the scale of fibonacci. Our minimum is 1, our maximum is 21. The higher, the more value or work a certain issue has.

Value : 1
Story Points: 2

CommentFileSizeAuthor
#17 2237829-17--query_alter_hooks_and_methods.patch11.41 KBdrunken monkey
#17 2237829-17--query_alter_hooks_and_methods--interdiff.txt402 bytesdrunken monkey
#15 2237829-15--query_alter_hooks_and_methods.patch11.42 KBdrunken monkey
#5 2237829-5--query_alter_hooks_and_methods.patch12.05 KBdrunken monkey
#1 2237829-1--query_alter_hooks_and_methods.patch3.2 KBdrunken monkey

Comments

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 2237829-1--query_alter_hooks_and_methods.patch3.2 KB

This patch would add that to the service class.

Additional argument in favor: Without this patch, this module doesn't define a single hook – and that's just un-Drupal-y! ;)

dmaciej’s picture

dmaciej commented

+1

but I think it should be fine to add a hook after adding orderBy() statements, just before executing the query.

dmaciej’s picture

dmaciej commented
Status: Needs review » Needs work
dmaciej’s picture

dmaciej commented

You should provide some hooks on the getFacets() method, to be able to alter the SQL queries for each facet too.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs work » Needs review
StatusFileSize
new 2237829-5--query_alter_hooks_and_methods.patch12.05 KB

Thanks a lot for your feedback!

Altering the final query, after sorts, can already easily be accomplished with the use of hook_query_TAG_alter() (specifically, hook_query_search_api_db_search_alter()).
Additionally, we could move adding the sorts into its own method, so it can more easily be overridden by subclasses.

I think the same route makes sense for the facets (and autocomplete) queries: we should just set appropriate tags and metadata there, too, so people have all the information they need for altering those.

Patch attached, please review!

Status: Needs review » Needs work

The last submitted patch, 5: 2237829-5--query_alter_hooks_and_methods.patch, failed testing.

drunken monkey’s picture

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

5: 2237829-5--query_alter_hooks_and_methods.patch queued for re-testing.

(Sorry, had a few un-pushed small fixes lying around locally.)

dmaciej’s picture

dmaciej commented

Altering facets using a query alter hook is really difficult and inefficient, because of the conditions structure. It should be possible to alter facets on the "flat" level, before it's added to the query.

drunken monkey’s picture

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

Don't really know what you mean with that? You can already alter the facets options that are passed in the query – where else would you want to alter facets?

Committed #5.

  • Commit 5caca2f on 7.x-1.x by drunken monkey: 
    Issue #2237829 by drunken monkey: Added a hook and methods for more...

Status: Fixed » Closed (fixed)

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

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Project: Search API Database Search » Search API
Version: 7.x-1.x-dev » 8.x-1.x-dev
Component: Code » Database backend
Status: Closed (fixed) » Patch (to be ported)

Needs to be ported to D8.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Issue tags: +Novice
nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Issue summary: View changes
Issue tags: +beta blocker
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 2237829-15--query_alter_hooks_and_methods.patch11.42 KB

Pretty straight-forward port. Just removed the postQuery() method since it's pretty pointless – you can just override search() itself.

AsadKamil’s picture

AsadKamil commented

Hi All,
It seems there is some whitespace error while applying patch to 8x branch.

root@asad-Vostro-3550:/var/www/html/git/search_api# git apply -v 2237829-15--query_alter_hooks_and_methods.patch 
Checking patch modules/search_api_db/search_api_db.api.php...
Checking patch modules/search_api_db/src/Plugin/search_api/backend/Database.php...
Checking patch modules/search_api_db/tests/src/Kernel/BackendTest.php...
Checking patch tests/search_api_test_db/search_api_test_db.module...
2237829-15--query_alter_hooks_and_methods.patch:279: new blank line at EOF.
+
Applied patch modules/search_api_db/search_api_db.api.php cleanly.
Applied patch modules/search_api_db/src/Plugin/search_api/backend/Database.php cleanly.
Applied patch modules/search_api_db/tests/src/Kernel/BackendTest.php cleanly.
Applied patch tests/search_api_test_db/search_api_test_db.module cleanly.
warning: 1 line adds whitespace errors.
root@asad-Vostro-3550:/var/www/html/git/search_api#
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 2237829-17--query_alter_hooks_and_methods--interdiff.txt402 bytes
new 2237829-17--query_alter_hooks_and_methods.patch11.41 KB

Thanks a lot for catching that!
Fixed in the attached revision of the patch.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

  • drunken monkey committed 5b5ea0c on 8.x-1.x 
    Issue #2237829 by drunken monkey: Added a hook to alter DB search...
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Excellent, thanks for reviewing!
Committed.
Thanks again, everyone!

Status: Fixed » Closed (fixed)

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