Problem/Motivation

I have a managed MySQL database from DigitalOcean. Their configuration has sql_require_primary_key set. The entities cannot be indexed as table creation fails since search_api_db_* does not specify a primary key.

Drupal\search_api\SearchApiException: SQLSTATE[HY000]: General error: 3750 Unable to create or change a table without a primary key, when the system variable 'sql_require_primary_key' is set. Add a primary key to the table or unset this variable to avoid this message. Note that tables without a primary key can cause performance problems in row-based replication, so please consult your DBA before changing this setting.: CREATE TABLE {search_api_db_roasts_roast_level} ( `item_id` VARCHAR(150) NOT NULL COMMENT 'The primary identifier of the item' ) ENGINE = InnoDB DEFAULT CHARACTER SET utf8mb4; Array ( ) in Drupal\search_api_db\Plugin\search_api\backend\Database->fieldsUpdated() (line 1122 of /workspace/web/modules/contrib/search_api/modules/search_api_db/src/Plugin/search_api/backend/Database.php).

Steps to reproduce

Create a database server on MySQL 8 with sql_require_primary_key set. Index values. Indexing fails.

Proposed resolution

Set item_id as a primary key?

Determine if the changes belong in DatabaseCompatibility\MySql

Or here directly https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/modules/sea...

      // For the denormalized index table, add a primary key right away. For
      // newly created field tables we first need to add the "value" column.
      if ($type === 'index') {
        $table['primary key'] = ['item_id'];
      }

I see that the primary key is set on the table at the end of the process

   // Add a covering index for field tables.
    if ($new_table && $type == 'field') {
      $this->database->schema()->addPrimaryKey($db['table'], ['item_id', $column]);
    }

Maybe we can set the initial primary key as item_id by default, and then at the end of the function drop it and add a new one of item_id and the column name.

Remaining tasks

CommentFileSizeAuthor
#19 search_api_required_1_index_field.patch3.34 KBostry.sn
#11 0396-patch-primary-key-search_api-module-contrib.patch1.73 KBostry.sn
#9 0396-patch-primary-key-search_api-module-contrib.patch1.93 KBostry.sn
#4 3195994-4--mysql_require_primary_key.patch1.58 KBdrunken monkey

Issue fork search_api-3195994

Command icon Show commands

Start 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:

About issue forks

Comments

mglaman created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue summary: View changes
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

For anyone else finding this: it's a "recent" change to the managed databases. See this Laravel issue thread: https://github.com/laravel/framework/issues/33238#issuecomment-647517431

A workaround would be adding this to the settings.php

$databases['default']['default']['init_commands'] = array(
  'primary_key' => "SET SESSION sql_require_primary_key=0"
);
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 3195994-4--mysql_require_primary_key.patch1.58 KB

Thanks a lot for reporting this issue!
I wasn’t aware of this setting, and it seems like a bad idea for installing a CMS where you have limited power over the created tables (and in some cases tables without primary key can make sense). So, probably the settings.php fix is a good idea in any case.
However, if we can avoid triggering the error in our code, that would still be nice, of course.

I am, on the other hand, not sure whether dropping a primary key with this setting enabled would work? Can you try with the attached patch and see what happens?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Yeah, I'll give it a try. I'll set up a new environment and point it to a DigitalOcean managed database. I got moving forward using the settings.php trick.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Any update on this?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Sorry, I haven't had much time. This is for a side project that I have not been able to give attention to due to normal work and client obligations.

mohamed.osama’s picture

mohamed.osama commented

Thanks so much mglaman for the workarount #3

I've added the below on the settings.php as I don't have access to mysql configuration.

$databases['default']['default']['init_commands'] = array(
  'primary_key' => "SET SESSION sql_require_primary_key=0"
);
ostry.sn’s picture

ostry.sn commented
StatusFileSize
new 0396-patch-primary-key-search_api-module-contrib.patch1.93 KB
ostry.sn’s picture

ostry.sn commented
ostry.sn’s picture

ostry.sn commented
StatusFileSize
new 0396-patch-primary-key-search_api-module-contrib.patch1.73 KB
ostry.sn’s picture

ostry.sn commented
smulvih2’s picture

smulvih2
Location Canada 🍁
commented

I am running into a similar issue using MySQL managed database service in Azure. When I try to save my index, or try to index content, I get this error:

Drupal\search_api\SearchApiException: Cannot add primary key to table 'search_api_db_INDEX_NAME_search_api_datasource_2': primary key already exists. in Drupal\search_api_db\Plugin\search_api\backend\Database->fieldsUpdated() (line 1187 of /var/www/html/modules/contrib/search_api/modules/search_api_db/src/Plugin/search_api/backend/Database.php).

I notice that it creates the table, but then fails on the primary key part. Then next time I try to index content or save the index, it creates new tables with _X appended (1, 2, 3.. n) like this search_api_db_INDEX_NAME_search_api_datasource_2.

Then I tried applying the patch in #11. With this I get the following error:

Drupal\search_api\SearchApiException: SQLSTATE[42000]: Syntax error or access violation: 1235 This version of MySQL doesn't yet support 'existing primary key drop without adding a new primary key. In @@sql_generate_invisible_primary_key=ON mode table should have a primary key. Please add a new primary key to be able to drop existing primary key.': ALTER TABLE "search_api_db_INDEX_NAME_search_api_datasource" DROP PRIMARY KEY; Array
(
)
 in Drupal\search_api_db\Plugin\search_api\backend\Database->fieldsUpdated() (line 1188 of /var/www/html/modules/contrib/search_api/modules/search_api_db/src/Plugin/search_api/backend/Database.php).

So I reverted the patch and tried adding the workaround in settings.php from #8. Then when I try to clear cache with drush cr and I get:

SQLSTATE[42000]: Syntax error or access violation: 1227 Access denied; you need (at least one of) the SUPER, SYSTEM_VARIABLES_ADMIN or SESSION_VARIABLES_ADMIN privilege(s) for this operation

I get the same error when I try to run SET SESSION sql_require_primary_key=0 directly in mysql.

drunken monkey opened merge request !110

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

@smulvih2: Thanks a lot for your feedback!
Then I guess we really have to make sure to always create tables already with the correct primary key. Created an MR for that, please test/review!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Would be great if some of you could test this to make sure this would now finally work reliably.

fte’s picture

fte commented

@smulvih2 I had the same issue with Azure service and I managed it thanks to the answer from the following post (configuring Azure instance) https://drupal.stackexchange.com/questions/316995/multiple-primary-key-d...

sonnykt’s picture

sonnykt
Location Melbourne, Australia
commented

@FTE
Confirming that turning sql_generate_invisible_primary_key off on Azure Flexible server fixed the issue on our site.

ostry.sn’s picture

ostry.sn commented
StatusFileSize
new search_api_required_1_index_field.patch3.34 KB

new version

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

@ostry.sn: Could you please try out the changes in my MR to see if they fix the problem for you, too?

The same goes for everyone else here. Did you try out the MR before changing the DB server settings? I don’t think it’s a good solution to let users stumble over this and then google until they can resolve it if we might be able to provide code that just works for everyone.

cassioalmeida’s picture

cassioalmeida commented

Hi everyone,

I was facing the issue with an Azure instance with the sql_generate_invisible_primpary_key ON. As the client has 300+ websites, turning off the flag was not an option.

I can confirm the MR worked fine! I was able to run pending tasks, re-index content, and so on.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

@cassioalmeida: Great to hear, thanks a lot for reporting back!

Anyone else who wants to still test the MR? Would be optimal to get a second confirmation.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

(I’d also love to run the tests with the other DBMSs, but unfortunately pipelines don’t seem to run correctly in issue forks, at least for this project.)

kieran.cott’s picture

kieran.cott
Location Manila 🇵🇭
commented

Thanks @ostry.sn. I had an issue with `sql_generate_invisible_primary_key` being set to "On" in my Azure DB - applying patch #19 has resolved this.

smulvih2’s picture

smulvih2
Location Canada 🍁
commented
Status: Needs review » Reviewed & tested by the community

Sorry took a while to test this MR as my project switched to a search_api_solr backend. Was able to test the MR and it works for me, no longer seeing unexpected database tables with _n added to the name, and the tables are being created correctly.

drunken monkey’s picture

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

@smulvih2: Good to hear, thanks a lot for reporting back!
Merged. Thanks again, everyone!

  • drunken monkey committed fb2a58c6 on 8.x-1.x 
    Issue #3195994 by drunken monkey, ostry.sn, smulvih2: Fixed error when...

Status: Fixed » Closed (fixed)

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