I'm trying to implement a "starts with" search when using Search API with database backend. It seems that this happens in Database.php on line 1947:
$db_or->condition('t.word', '%' . $this->database->escapeLike($word) . '%', 'LIKE');

I'm not sure if it's possible to alter the query in a hook or by overriding some method of some class.

But would it make sense to add an option to the Database server itself? Maybe another checkbox under "Search on parts of a word", something like Use "Starts with" to avoid ambiguous results or similar. With that selected the first "%" would be removed from the condition:
$db_or->condition('t.word', $this->database->escapeLike($word) . '%', 'LIKE');

I can provide a patch if this makes sense. Or is there a way to override this per view or index for example?

Comments

mErilainen created an issue. See original summary.

merilainen’s picture

Title: Add an option for "starts with" for partial martching » Add an option "starts with" for partial matching
drunken monkey’s picture

Thanks for proposing this new feature!
I think it would make most sense to change the existing option to radios instead of a checkbox: does the user want whole-word matching, prefix matching or complete substring matching?
We'd need an update hook for that, then, but that should be simple enough. A bit more work would be the test coverage to ensure this works properly, and stays working. But all in all, not that hard, I think, and definitely something a lot of people might want. So, thanks again! If you could provide a patch, that would be great!

For doing this in a custom module, you could probably just use hook_search_api_db_query_alter() to alter the query accordingly. Would need a bit of searching, though, to reliably find all such fulltext conditions – our queries tend to get pretty complex.

Configuring this per-view or per-index would be a lot more work, that's why we also don't allow this for the current "partial matching" setting.
It would definitely be possible in some way, but I'd stay with this for now, unless you really need it.
(Per-index would be trivial to do already, though, by just having one search server per index. For a DB server, there's more or less no additional cost involved for creating multiple servers, after all.)

merilainen’s picture

I was thinking about the radio button element too, but it seems it would be a lot easier to implement an additional (conditional) checkbox element. It wouldn't break any exiting configuration or tests and changes to code would be minimal. Maybe logical naming would make it usable? Something like Match only words starting with given keywords.

merilainen’s picture

Assigned: Unassigned » merilainen
Status: Active » Needs work
StatusFileSize
new4.19 KB

Here is a first stab with the additional checkbox approach, seems to work. Didn't include the condition for the checkbox yet, but that should be trivial.

merilainen’s picture

Here is a patch with the conditions for the form elements. Also added starts_with to the Database search server schema.

merilainen’s picture

Fixed double key, doh.

drunken monkey’s picture

Issue tags: +Needs tests

Thanks a lot for the patch, looks pretty good!
However, in general, when posting new revisions of an existing patch, please include an interdiff. (It wasn't really necessary here, since I didn't look at the issue until the last patch was there, but probably still better to be on the safe side.)

Changing less code and not requiring and update hook are of course good arguments. However, the UI does not have to match the internal configuration – we can just have three radios in the UI, but use the same two boolean config keys as you currently do internally. We just need a submitConfigurationForm() implementation to bridge the two.
So, I don't know what the third follower's take on this is, but I think I would prefer radios there.
Also, an update hook and a few changed config files (which you already changed anyways) and tests wouldn't be that bad, either – and maybe not as confusing for developers.
I'm not sure which I prefer, to be honest.

In any case, disabling the first option once the second is enabled isn't a pattern we normally use, I think. When the user unchecks "Partial matching" and the second option gets hidden, that should usually be enough to clue them in that only enabling the second option doesn't work.
But, as said, please change to radios, unless you're strongly opposed.

Also, viewSettings() could be adapted to show the matching strategy in a single item in any case. That's completely independent of the UI or the internal options.

+++ b/modules/search_api_db/src/Plugin/search_api/backend/Database.php
@@ -1888,6 +1910,7 @@ class Database extends BackendPluginBase implements PluginFormInterface {
     $keyword_hits = [];

NOOO, it breaks the pretty pattern! T___T
But since that variable is also just used once, we might as well inline it … (Phew!)

And, finally, we'll still need test coverage for this new feature.
Just a few lines in \Drupal\Tests\search_api_db\Kernel\BackendTest should suffice – like editServerPartial()/searchSuccessPartial(), maybe just right after those.

drunken monkey’s picture

merilainen’s picture

I changed the form now to use radio buttons, added submitConfigurationForm() and changed viewSettings() to a single item. Also added editServerStartsWith() to BackendTest.php but I'm not familiar with writing tests so I would need some help with adding the test for that "starts with" matching. Included an interdiff this time too.

drunken monkey’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new8.02 KB
new10.78 KB

Thanks a lot, looks great!
I think we can keep the code a bit more concise, though – especially in submitConfigurationForm() – see the attached patch. (Your version also failed to set all the other config values, or call the parent method.)
This, by the way, also adds the required test method – I just copied searchSuccessPartial() and adapted the expected return values and/or keywords based on the different semantics. (Worked surprisingly well, I didn't even need to consult \Drupal\Tests\search_api\Functional\ExampleContentTrait::insertExampleContent() on the actual contents of the indexed test entities.)

Looking at the code, though, I begin to think we should really go with the string config value and update function – I'll post an adapted patch soon.

Status: Needs review » Needs work

The last submitted patch, 11: 2971033-11--db_backend_starts_with.patch, failed testing. View results

drunken monkey’s picture

Status: Needs work » Needs review
StatusFileSize
new10.22 KB
new12.37 KB

And here is the promised adapted version with just a single option value.
I think we'll be happier with that going forward, since otherwise it's always a bit of a mental challenge converting between the UI options and the internal config values.
Anyone else want to weigh in on this?

I'd also be interested in other people's opinions of the radio button labels:

+++ b/modules/search_api_db/src/Plugin/search_api/backend/Database.php
@@ -465,11 +466,26 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
+        'words' => $this->t('Match whole words only'),
+        'partial_matches' => $this->t('Match on parts of a word'),
+        'starts_with' => $this->t('Match words starting with given keywords'),

(The test "fail" is a testbot bug: #2973992: Permission issue in Nightwatch step marks all full testruns as unstable.)

Status: Needs review » Needs work

The last submitted patch, 13: 2971033-13--db_backend_starts_with.patch, failed testing. View results

merilainen’s picture

This looks good to me and thanks for quick response! I won't mark this RTBC if you want to get a review from a fresh pair of eyes.

drunken monkey’s picture

Status: Needs work » Needs review

Yes, another review would be great, it is a larger change.

drunken monkey’s picture

Assigned: merilainen » Unassigned
borisson_’s picture

Status: Needs review » Reviewed & tested by the community

I haven't manually tested this, but the code looks great. Setting to rtbc based on that + the review in #15.

drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Great to hear, thanks a lot for reviewing!
Committed.
And thanks a lot again, mErilainen!

Status: Fixed » Closed (fixed)

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