The table echo_push_subscription was added with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/597135, which is REL1_35. At that point there exists only one foreign key.
The second foreign key was added with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/625928 in REL1_36.

But the second patch set for does not add the foreign key as part of the updater which should be dropped now. The second foreign key only exists in the inital schema file.
So older wikis running the 1.35 update seems affected, because the foreign key does not exists there.

Have no idea at the moment how to detect this from the updater.

SQLite cannot drop foreign keys, so it just clones and replaces the table instead, which means it's not affected. Postgres doesn't even have this patchfile, I guess because it wasn't supported at the time. So it's only a problem for MySQL / MariaDB. (MariaDB supports DROP FOREIGN KEY IF EXISTS but I suppose that's not much help here.) I guess the best way is to just split the drop command into its own file, try to do it and swallow the error?

So I checked my email again and it seems like i need to add this tag as I tested on 1.39

Moving to main growth team board. Not sure if there is anything actionable for Growth-Team on this one.

I just did a test upgrade from 1.37.4 to 1.39.0-rc.1 (latest commits on REL1_39 for mediawiki core and the Echo extension) and had this issue as well.

I disabled the extension and completed the update for everything else, but what should I do to make sure the Echo update is properly complete? Can I remove the DROP FOREIGN KEY from the script somehow?

Maybe undo rECHO90639411fa450f4ef81984ac937b442c26b1b977 this for temporal fix for user side. But you should use "MariaDB 10.2" or later. This was done due to combability issue with MySQL according to T314779. I'm just waiting for patch.

I think think the action would be to split the drop command into a separate file and wrap it in a try-catch. IMO we should fix this ASAP as 1.39 is pending release and that's when most people would encounter this bug.

Change 858839 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/core@master] Fix Echo echo_push_subscription_ibfk_2 database inconsistency

https://gerrit.wikimedia.org/r/858839

Change 858840 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/Echo@master] Fix echo_push_subscription_ibfk_2 database inconsistency

https://gerrit.wikimedia.org/r/858840

Patching core to fix Echo seems too drastic to me. I think there are ways to handle this inside Echo. Give me a bit of time.

In T322143#8408874, @Ladsgroup wrote:

Patching core to fix Echo seems too drastic to me. I think there are ways to handle this inside Echo. Give me a bit of time.

Thanks for looking into it! I agree it's not great (although it could be removed in the next release). It would be great to have a fix before/around the 1.39 release though. As things stand now, the DB updater will break for everyone who is trying to upgrade from the previous LTS and uses Echo.

There are a couple of things you can do:

• You can make it add all of the updates inside echo. E.g. something like this in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/858840/2/includes/Hooks.php:

if (
	$dbType === 'mysql' &&
	!$updater->getDB()->indexExists( 'echo_push_subscription', 'eps_user', __METHOD__ ) &&
	
) {
	$table = $updater->getDB()->tableName( 'echo_push_subscription' );
	$updater->getDB()->query("SHOW CREATE TABLE $table;", __METHOD__);
	$needed = false;
	foreach ( $res as $row ) {
		if (strpos($row[1], 'echo_push_subscription_ibfk_2')) {
			$needed = true;
		}
	}
	if ( $needed ) {
		$updater->addExtensionUpdate( [
		'applyPatch',
		$dir . '/mysql/patch-cleanup-push_subscription-foreign-keys-indexes-2.sql',
	] );
	}
}

(Expand or change as necessary, untested)

• • you can do all sorts of complicated stuff like simply calling applyPatch, while that's not a good practice, I think it's okay to do for a temporary fix, you're definitely allowed to make read queries to check the structure and see if the patch is needed
• If you didn't like the previous one, you're gonna love this one. Pull off a sqlite, make a temp table, move data around, drop the old one and rename the temp to current. That way you're sure it's not going to error out.
• You could possibly do some magic in SQL though information schema: Stuff like https://stackoverflow.com/questions/32320381/how-do-i-use-an-if-statement-with-an-alter-statement-in-mysql
  • But highly discouraged.

In T322143#8410928, @Ladsgroup wrote:

• You can make it add all of the updates inside echo. E.g. something like this in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/858840/2/includes/Hooks.php:

Thanks, that looks like a good approach. I checked a couple SHOW commands but did not think of SHOW CREATE TABLE.

• you can do all sorts of complicated stuff like simply calling applyPatch

You can't, applyPatch and basically every other method that actually does something has been changed to protected in the wake of T157651: sql.php must not run LoadExtensionSchemaUpdates.

• You could possibly do some magic in SQL though information schema: Stuff like https://stackoverflow.com/questions/32320381/how-do-i-use-an-if-statement-with-an-alter-statement-in-mysql

I considered that but wasn't sure if it's same to assume that the MediaWiki dbadmin user can read information_schema. The defaults seem fairly permissive though, so I guess this would be another good option.

Change 861935 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/Echo@master] schema: Drop echo_push_subscription_ibfk_2 separately

https://gerrit.wikimedia.org/r/861935

In T322143#8410928, @Ladsgroup wrote:

There are a couple of things you can do:

• You can make it add all of the updates inside echo. E.g. something like this in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/858840/2/includes/Hooks.php:

if (
	$dbType === 'mysql' &&
	!$updater->getDB()->indexExists( 'echo_push_subscription', 'eps_user', __METHOD__ ) &&
	
) {
	$table = $updater->getDB()->tableName( 'echo_push_subscription' );
	$updater->getDB()->query("SHOW CREATE TABLE $table;", __METHOD__);
	$needed = false;
	foreach ( $res as $row ) {
		if (strpos($row[1], 'echo_push_subscription_ibfk_2')) {
			$needed = true;
		}
	}
	if ( $needed ) {
		$updater->addExtensionUpdate( [
		'applyPatch',
		$dir . '/mysql/patch-cleanup-push_subscription-foreign-keys-indexes-2.sql',
	] );
	}
}

I have used that idea and created a patch set

In T322143#8388684, @Tgr wrote:

I think think the action would be to split the drop command into a separate file and wrap it in a try-catch. IMO we should fix this ASAP as 1.39 is pending release and that's when most people would encounter this bug.

Yes, I already encounter this bug while upgrade to 1.39 (GA released).

Same problem while trying to upgrade from 1.38.2 to 1.39.0.

Change 858840 abandoned by Gergő Tisza:

[mediawiki/extensions/Echo@master] Fix echo_push_subscription_ibfk_2 database inconsistency

Reason:

Per T322143#8408874 and followup comments

https://gerrit.wikimedia.org/r/858840

Change 858839 abandoned by Gergő Tisza:

[mediawiki/core@master] Fix Echo echo_push_subscription_ibfk_2 database inconsistency

Reason:

Per T322143#8408874 and followup comments

https://gerrit.wikimedia.org/r/858839

In T322143#8360921, @Umherirrender wrote:

The table echo_push_subscription was added with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/597135, which is REL1_35. At that point there exists only one foreign key.
The second foreign key was added with https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/625928 in REL1_36.

But the second patch set for does not add the foreign key as part of the updater which should be dropped now. The second foreign key only exists in the inital schema file.
So older wikis running the 1.35 update seems affected, because the foreign key does not exists there.

Additional information, the new field in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/618128 also missed updater support, it is also part of REL1_36, so REL1_35 wikis needs it as well (added to the same patch set).
The new index from https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/619570 is also missing support, it is also part of REL1_36, so REL1_35 wikis needs it as well (support added to the same patch set).

Change 861935 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] schema: Run cleanup updates for echo_push_subscription separately

https://gerrit.wikimedia.org/r/861935

Change 869711 had a related patch set uploaded (by Gergő Tisza; author: Umherirrender):

[mediawiki/extensions/Echo@REL1_39] schema: Run cleanup updates for echo_push_subscription separately

https://gerrit.wikimedia.org/r/869711

Thanks for the fix @Umherirrender! Backported to 1.39. Echo is not bundled with the core tarball so AFAIK it doesn't really have any concept of a release, people are just expected to use the head of the REL1_XX branch. So this shouldn't affect users anymore.

Change 869711 merged by jenkins-bot:

[mediawiki/extensions/Echo@REL1_39] schema: Run cleanup updates for echo_push_subscription separately

https://gerrit.wikimedia.org/r/869711

Change 869767 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Echo@master] patch-echo_push_subscription-add-column-eps_topic.sql: Add table prefix

https://gerrit.wikimedia.org/r/869767

Change 869767 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] patch-echo_push_subscription-add-column-eps_topic.sql: Add table prefix

https://gerrit.wikimedia.org/r/869767

Change 869792 had a related patch set uploaded (by Umherirrender; author: Reedy):

[mediawiki/extensions/Echo@REL1_39] patch-echo_push_subscription-add-column-eps_topic.sql: Add table prefix

https://gerrit.wikimedia.org/r/869792

Change 869792 merged by jenkins-bot:

[mediawiki/extensions/Echo@REL1_39] patch-echo_push_subscription-add-column-eps_topic.sql: Add table prefix

https://gerrit.wikimedia.org/r/869792