I am getting the following error message when performing the database update after upgrading from 8.x-3.0-beta4 to 8.x-3.0-beta5.

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 "feeds_clean_list" DROP PRIMARY KEY; Array

Is anybody else having the same issue or is it just a problem with my version of MySQL server (v8.0)?

Issue fork feeds-3454534

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:

Comments

torfj created an issue. See original summary.

torfj’s picture

Issue summary: View changes
megachriz’s picture

It appears to have something to do with a setting on MySQL, one that is called "sql_generate_invisible_primary_key". I read that if that is set to 0, then you should be good.
https://github.com/prisma/prisma/issues/20944
https://magento.stackexchange.com/questions/370387/mysql-error-mysql-doe...

I don't know how the issue should be fixed in an other way yet.

KarlShea made their first commit to this issue’s fork.

karlshea’s picture

Status: Active » Needs review

Some searching seems like it might work to do this inside a transaction? Untested MR created.

torfj’s picture

Status: Needs review » Needs work

I tested the MR and I am still getting the same error.

karlshea’s picture

Tested, it doesn't work. I don't think having sql_generate_invisible_primary_key enabled is compatible with what we have available to modify the schema in Drupal.

karlshea’s picture

torfj’s picture

Status: Needs work » Needs review

I tried creating a new table with the desired primary key, coping the data from the old table to the new one, and then renaming the new table to the old one. This works for me, but it might be a better idea to update the sql_generate_invisible_primary_key value.

karlshea’s picture

Status: Needs review » Needs work

Unfortunately the web user would need the SESSION_VARIABLES_ADMIN privilege to change the sql_generate_invisible_primary_key value.

ericgsmith’s picture

I think approach looks good - added a comment to the MR as its using incorrect column definition.

torfj’s picture

Status: Needs work » Needs review
karlshea’s picture

Status: Needs review » Reviewed & tested by the community

Oh good idea! I tested this with sql_generate_invisible_primary_key enabled and it worked just fine.

ericgsmith’s picture

+1 from me

megachriz’s picture

Status: Reviewed & tested by the community » Needs work

This looks like a great solution, however calling a function that returns the current schema (in this case a call to feeds_schema()) is discouraged. If for whatever reason in the future the table "feeds_clean_list" would get removed, the update function would break. Update functions should never rely on the current implementation of hook_schema().

From https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

Never assume that the database schema is the same when the update will run as it is when you wrote the update function. So, when updating a database table or field, put the schema information you want to update to directly into your function instead of calling your hook_schema() function to retrieve it (this is one case where the right thing to do is copy and paste the code).

karlshea’s picture

Status: Needs work » Needs review

Ahhh right, commit pushed with a static table definition.

torfj’s picture

Status: Needs review » Reviewed & tested by the community

I tested with the static table definition and it still works. Thanks for pointing that out.

ericgsmith’s picture

+1 testing and works for me

Apologies @torfj for the bad advice - thank you @MegaChriz for picking that up and thank you @KarlShea for updating.

  • MegaChriz committed ce7aded3 on 8.x-3.x authored by KarlShea
    Issue #3454534 by KarlShea, torfj, ericgsmith, MegaChriz: Fixed SQL...
megachriz’s picture

Status: Reviewed & tested by the community » Fixed

Thanks all! I merged the code!

Status: Fixed » Closed (fixed)

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