Problem/Motivation

Upon trying to configure Search API with the database as a backend (for testing purposes), I've enabled the search_api and search_api_db modules.

On the test server (running MySQL 5.6), I have several databases (for different sites in a multisite). Creating a server and the corresponding index works well. When using the default database, I can index just fine. But when using any of the other available databases, things fail.

E.g. let's take the test database above as an example:

At /admin/config/search/search-api/server/database_search, I have a message saying "There is currently 1 task pending for this server." - When clicking "Execute tasks now", there's a WSOD with only "The website encountered an unexpected error. Please try again later."

In the logs, I see:

Drupal\search_api\SearchApiException while adding index test to server test: SQLSTATE[HY000]: General error: 1709 Index column size too large. The maximum column size is 767 bytes.: ALTER TABLE {search_api_db_test_search_api_datasource} ADD PRIMARY KEY (`item_id`, `value`); Array ( ) in Drupal\search_api_db\Plugin\search_api\backend\Database->fieldsUpdated() (line 1044 of /mnt/www/html/test/docroot/modules/contrib/search_api/modules/search_api_db/src/Plugin/search_api/backend/Database.php).

Similarly, at /admin/config/search/search-api/index/test, when clicking "Index now", it returns:

Couldn't index items. Check the logs for details.

And in the log files I see:

Drupal\search_api\SearchApiException: SQLSTATE[HY000]: General error: 1709 Index column size too large. The maximum column size is 767 bytes.: ALTER TABLE {search_api_db_test_search_api_datasource_1} ADD PRIMARY KEY (`item_id`, `value`); Array ( ) in Drupal\search_api_db\Plugin\search_api\backend\Database->fieldsUpdated() (line 1044 of /mnt/www/html/test/docroot/modules/contrib/search_api/modules/search_api_db/src/Plugin/search_api/backend/Database.php).

And

Drupal\search_api\SearchApiException while trying to index items on index test: Could not index items on index 'test' because pending server tasks could not be executed. in Drupal\search_api\Entity\Server->indexItems() (line 341 of /mnt/www/html/test/docroot/modules/contrib/search_api/src/Entity/Server.php)

Proposed resolution

TBD

Remaining tasks

Identify root cause of the issue.

User interface changes

TBD

API changes

TBD

Data model changes

TBD

CommentFileSizeAuthor
#4 2922874-4--proper_compatibility_handler.patch7.58 KBdrunken monkey
#4 2922874-4--proper_compatibility_handler--tests_only.patch4.24 KBdrunken monkey
#4 2922874-4--proper_compatibility_handler--interdiff.txt5.63 KBdrunken monkey
#2 2922874-2--proper_compatibility_handler.patch4.67 KBdrunken monkey
search_api_db_backend.png43.71 KBanavarre

Comments

anavarre created an issue. See original summary.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 2922874-2--proper_compatibility_handler.patch4.67 KB

Thanks for reporting!
I now see that the code handling DBMS compatibility adaptions won't really work if the user picks a non-default database for the server. Apparently, I didn't see how we could avoid this – or maybe I just didn't want to bother. Anyways, I think the fix should be pretty simple – please see and test the attached patch!

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented 
+++ b/modules/search_api_db/src/DatabaseCompatibility/GenericDatabase.php
@@ -59,5 +68,4 @@ public function preprocessIndexValue($value, $type = 'text') {
   }
-
 }

This is unrelated, and wrong, the empty line should stay.

Is it possible to test this? Or does this need the actual second database to exist on the server?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 2922874-4--proper_compatibility_handler--interdiff.txt5.63 KB
new 2922874-4--proper_compatibility_handler--tests_only.patch4.24 KB
new 2922874-4--proper_compatibility_handler.patch7.58 KB

Ah, you're right, good catch! Thanks for that!
The move of the getDatabase() method also doesn't make any sense … Apparently, even for me there's a "too drunken". ;)

And good point about the tests! Unfortunately, just inventing connection info doesn't seem to work, this throws an exception as soon as you actually try to retrieve the connection. (Makes sense, I guess, but bad for us.) So, to properly test this for connections with a different DBMS than the default, we'd probably need to do a Unit test and mock the shit out of this. (If that's even possible …)
However, I looked around and saw a neat trick in Core for at least testing with a clone of the default connection. This will be treated internally as a completely separate connection (even if the details are identical), so that's at least something. It will at least cover the issue reported here which was, after all, connections with the same DBMS.

Patch with tests attached – let's see if it really passed on all three DBMSs …

PS: If you're wondering about the moved getModuleHandler() method – that's just the move of getDatabase() back to its original location.

The last submitted patch, 4: 2922874-4--proper_compatibility_handler--tests_only.patch, failed testing. View results

borisson_’s picture

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

Looks good!

  • drunken monkey committed 89ce1cf on 8.x-1.x 
    Issue #2922874 by drunken monkey, borisson_: Fixed DBMS compatibility...
drunken monkey’s picture

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

Good to hear, thanks again for reviewing!
Committed.

Status: Fixed » Closed (fixed)

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