Problem/Motivation

There is a problem with Drupal\search_api_db\Plugin\search_api\backend\Database.

If the default table name for a text table - i.e. search_api_db_[index_name]_text - is already taken in the database then Database::findFreeTable() will return e.g. search_api_db_[index_name]_text_1.

However, when indexing the items Database::indexItem() tries to compute the text table name by doing $text_table = $denormalized_table . '_text';. This does not match the previously generated table name. $denormalized_table is e.g. search_api_db_[index_name]_1 at this point.

So to summarize:
search_api_db_[index_name]_text_1 gets created.
search_api_db_[index_name]_1_text gets queried.

This makes the indexing fail completely.

Proposed resolution

Remaining tasks

User interface changes

API changes

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 : 2
Story Points: 3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

Issue summary: View changes

Fixing issue summary.

Nick_vh’s picture

Issue summary: View changes
Issue tags: +beta blocker
drunken monkey’s picture

Seems to be fixed already. But I guess we can add a test for it.

tstoeckler’s picture

Status: Needs review » Needs work

Hmm.. to actually test the behavior described in the IS, the test would also have to create a search_api_db_database_search_index_text table, as far as I can tell. Otherwise, looks good.

  • drunken monkey committed 5bce36c on 8.x-1.x
    Issue #2492653 by drunken monkey: Added tests for Database::...
drunken monkey’s picture

Status: Needs work » Fixed

Hmm.. to actually test the behavior described in the IS, the test would also have to create a table, as far as I can tell.

For exactly the scenario in the IS, yes; but I think this one will catch even more potential errors, or at least should catch the one mentioned just as well.
So, committed.
Thanks for your review!

Status: Fixed » Closed (fixed)

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