Update hook 8102 should check for the existence of the field before trying to add it

Here is a patch that applies to 2.1.3. MR should be considered the primary fix (contents of 8102 changed between 2.1.3 and the latest 2.1.x branch).

I'm inclined to say no to this. If you have modified your database in a way that makes the hook fail, then I think it SHOULD fail as a way to inform you that you have something that needs to be looked at and fixed on your site before updating. Just skipping the update does not ensure that your site will end up in the state that this module expects.

If you have patches to this or any other module applied on your site, then the update process always has a manual component to reconcile your local changes with the module changes. In this case, you're informed by the update hook failure that your database needs to be examined and you need to ensure that the schema you're using is the same as what the update hook is trying to establish then manually change the database schema on your site if it is not. Then you can manually update the honeypot version in the system.schema config to skip the update if that is appropriate for your site.

In short, if your site's code or database has been modified and is not as expected, then the update hook can't know how to fix it so it should fail. The update hook should not just ignore this condition, which is what your patch would do.

BTW the only reason fieldExists() was used in hook_update_8101() is because hook_update_8100() was faulty and did not run on some sites. Therefore hook_update_8101() needed to test to see if the previous changes had actually worked and then re-run those changes if they hadn't worked. That's not the case with the current issue.

OK, that does not sound unreasonable. Thanks also for the clarification about 8100/8101.

What if you didn't make any manual changes to your database, but there's still a problem? I get:

> Syntax error or access violation: 1075 Incorrect table definition; there can be only one auto column and it must be defined as a key: ALTER TABLE "honeypot_user" ADD `id` INT NOT NULL auto_increment COMMENT 'Unique record ID.', ADD PRIMARY KEY (`id`);

when I try to run the update. I have never manually modified the database table.

@pukku by chance are you using MySQL 8 with sql_generate_invisible_primary_key (GIPK) mode enabled? That MySQL setting seems to be a case where this update doesn't work (because it's not properly supported by drupal core yet)

Hah! Ok, that is turned on. This is a database provided by a parent department, so I don't have any control over the settings.

I was able to make it eventually work by using the "execute php" part of Devel, and uninstalling the schema, installing the schema, and then using some system call I found online to tell Drupal that I was at the correct patch level.

But I'm glad to learn about this, in case it comes up elsewhere...

Thanks,
Ricky

I created 📌 Support MySQL GIPK mode Needs review for working on GIPK support.