When using search_api_db as the backend, under some conditions the pertinence score for a keyword search is not calculated correctly, resulting in erroneous ordering of search results per pertinence.
It happens when the search index is configured with multiple (full) text fields, with multiple fields containing the same boost factor, and when making a search query that matches a word contained in multiple fields with the same boost factor.
It happens because Drupal\search_api_db\Plugin\search_api\backend::createKeysQuery() builds a query with a "GROUP BY item_id, score". If multiple rows have the same score for the same item, only one of them will be retained when calculating the final pertinence score.
In this case, Node 1 will have 2 rows with the same score for "keyword" (each of them = 5000), and Node 2 will have 2 rows with different scores (5000 and 500), due to the boost factor. Due to the buggy GROUP BY, Node 1 ends up with a score of 5000 (instead of 10'000), and Node 2 ends up with a score of 5500.
Replacing the "GROUP BY score" (which makes no sense) with a "GROUP BY field_name", which will ensure that 2 fields with the same boost factor containing the same word will be calculated correctly.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !11
Comments
Comment #3
mdupontComment #4
drunken monkeyThanks a lot for reporting this problem and attempting a fix.
However, please use patches in this project instead of issue forks, as the d.o team has been unable/unwilling to get issue fork testing working for it yet. (See #3190024: Problem with test dependencies when testing issue forks.)
Running the tests manually for your patch, however, shows that it leads to incorrect SQL queries in some scenarios. However, I think I was able to determine the correct fix – the problem was that we didn’t sum the score in all the right scenarios. The code was never meant to group by score in any scenario. I now added a comment to make that clear.
However, I’m unfortunately still unable to reproduce this problem. As you see, I tried to write a regression test for it, but couldn’t get it to fail with the unpatched version. Are you maybe able to get it to work? That would be the last thing to do before I can commit it, I think.
(What I can see pretty clearly from the code, I think, is that the bug will only trigger if the match mode is not set to
'words', i.e., partial matching is enabled.)Of course, please also test/review the patch so far and see if it also fixes the problem.
Thanks again!
Comment #5
drunken monkeyNW for the missing/incomplete regression test.
Comment #6
mdupontThanks for your useful feedback. I'm trying to come up with a regression test that triggers this issue.
In the meantime, I confirm I was able to reproduce the issue on a stock Drupal install with Search API 8.x-1.x-dev (latest git commit), with a simple setup:
Using Webprofiler, we see that the view performs the following SQL query:
SELECT "t"."item_id" AS "item_id", SUM(t.score) AS "score" FROM (SELECT "t"."item_id" AS "item_id", "t"."score" AS "score", CASE WHEN t.word LIKE '%testkeyword%' THEN 1 ELSE 0 END AS "w1" FROM "search_api_db_ind_text" "t" WHERE ("t"."word" LIKE '%testkeyword%' ESCAPE '\') AND ("t"."field_name" IN ('body', 'title')) GROUP BY t.item_id, t.score, w1) "t" GROUP BY t.item_id ORDER BY "score" DESC LIMIT 11 OFFSET 0We can check that running this query in the DB results in both nodes having the same score.
Now, if we alter the SQL query just a bit to add a
GROUP BY t.field_nameas such:SELECT "t"."item_id" AS "item_id", SUM(t.score) AS "score" FROM (SELECT "t"."item_id" AS "item_id", "t"."score" AS "score", CASE WHEN t.word LIKE '%testkeyword%' THEN 1 ELSE 0 END AS "w1" FROM "search_api_db_ind_text" "t" WHERE ("t"."word" LIKE '%testkeyword%' ESCAPE '\') AND ("t"."field_name" IN ('body', 'title')) GROUP BY t.item_id, t.field_name, t.score, w1) "t" GROUP BY t.item_id ORDER BY "score" DESC LIMIT 11 OFFSET 0This time when we run the query we see node 2 having a score of 2000 and node 1 having a score of 1000, as expected.
Back to trying to correctly write a regression test for this.
Comment #8
mdupontTesting whether an updated regression test does trigger the regression.
The patch in #4 was not working because items were not marked for reindexing after changing the boost factor for the "name" field, so the call to
$this->indexItems()was doing nothing and the query results were incorrect.Comment #9
mdupontThis patch for a regression test should work. The generated query in previous patches was incorrectly sorting results.
Comment #10
mdupontTest only patch should fail, the other one should pass.
Test failures on D9.3 are due to the state of Drupal core 9.3.x-dev and not due to this patch.
Comment #12
mdupontBack to NR as test failures were expected.
Comment #14
mdupontBundled a fix for the failing tests on Drupal 9.3, even if distinct from this issue.
Comment #16
mdupontForgot to fix one call to Role::create().
Comment #17
mdupontIn addition to passing the tests, I confirm the patch in #16 also fixes the issue on a real Drupal website performing searches via Views.
Comment #19
drunken monkeyThanks a lot, great work! And sorry for taking so long to respond.
We fixed the test problems with Drupal 9.3 already, so I used your patches from #10.
I just made some slight changes and then committed. Thanks again!