[00:38:26] anyone around to give this revert a +2 for immediate deployment? https://gerrit.wikimedia.org/r/#/c/375095/ [00:39:04] the change was broken in a pretty ridiculous way [00:40:03] I thought we generally just +2'd on reverts? ;) [00:40:07] done anyway [00:40:19] thanks [00:40:52] Just made a cherry pick, want that CR+2'd too? [00:41:10] is my internet laggy or is it hte bot... [00:41:14] the bot seemingly [00:41:48] heh [00:41:49] I +2'd it [00:55:20] I'll deploy [00:58:03] I'm not sure why wikibugs is lagging [16:41:39] anomie: I just noticed the log_search PK is conflicting with an old updater... [16:41:44] https://gerrit.wikimedia.org/r/375401 [16:42:56] Hmm [16:43:04] Looks like it should go from sqlite too [16:46:56] Reedy: I don't know about dropping it entirely. What happens then if someone is upgrading from an old enough MW that the index doesn't exist? [16:47:15] The table was new in 1.16 [16:47:51] https://github.com/wikimedia/mediawiki/blob/master/maintenance/archives/patch-log_search.sql [16:47:56] Created it without a PK, but with the unique [16:48:20] https://github.com/wikimedia/mediawiki/blob/master/maintenance/archives/patch-log_search-rename-index.sql drops the PK (?) and adds the unique index [16:49:18] So, unless the 1.16 cycle was really screwed up... [16:49:21] They should have one or the other [16:49:26] Some might have both (still)... [16:50:09] Reedy: From the comments in patch-log_search-rename-index.sql, it seems that it had the PK instead of the UNIQUE between r50567 and r51465, both during the 1.16 development cycle presumably. [16:50:51] So yeah, it looks like it should have one or the other. [16:51:22] It *could* be worth adding a patch that just runs after the PK migration, to drop it if it exists... [16:51:37] Based on that sqlite update file, sqlite might have both since it never dropped the PK. [16:52:52] Possibly, yeah [16:53:06] Cheers [16:53:15] I've got a task about creating a schema-differ thing [16:53:32] So we can look more wholelly if we've these edge cases we really need to sort for people [16:53:42] 'renameIndex' will complain and skip if both the old and new indexes exist. Probably not worth worrying about, even though sqlite people will now have both indexes if they ran the updater since your PK patch. [22:54:04] anomie, is class_exists( CommentStore::class ) really needed in extensions? most WMF-maintained ones only work with master MW anyway