Repository navigation
fix(db): don't hint missing ban indexes in PruneBans (#1578) - #1581
Open
rumblefrog wants to merge 2 commits into
Open
rumblefrog wants to merge 2 commits into
rumblefrog wants to merge 2 commits into
Conversation
2.2.1's PruneBans() used FORCE INDEX (type_authid / type_ip) on :prefix_bans. Long-upgraded installs can lack those indexes, and a hint on a missing index is MariaDB error 1176, which fataled the banlist, servers, and dashboard pages. Drop the hints and add updater migration 811 that creates either index when missing, so upgraded installs converge with struc.sql.
Combine missing-index ALTERs into one statement, correct the list of affected surfaces (banlist + add/edit ban, not home/servers), add the type_ip error variant to the troubleshooting entry, and tighten the AGENTS.md rationale for the comms FORCE INDEX exception.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1578.
Problem
#1577 (2.2.1) added
FORCE INDEX (type_authid)/FORCE INDEX (type_ip)to the submission lookup inPruneBans(). Some long-upgraded installs don't have those two indexes on:prefix_bans.702.phpcreates them, but the updater only runs scripts above the storedconfig.version, so an install that moved past 702 without running it never got them. MariaDB treats a hint on a missing index as error 1176, not a no-op. As a result, the Ban List page and the add / edit ban flows (PruneBans()callers:page.banlist.php,admin.edit.ban.php,api_bans_add) fatal with:Fix
PruneBans(): drop the index hints. When the indexes exist, the optimizer picks them for each arm on its own. The reviewer checked this against a 200k-ban table afterANALYZE.web/updater/data/811.php(registered instore.json): checksinformation_schema.STATISTICSand adds whichever oftype_authid/type_ipis missing, in a singleALTER TABLE. This makes upgraded installs matchstruc.sql. It's idempotent and portable (no MariaDB-onlyADD INDEX IF NOT EXISTS), and it's a no-op on fresh installs, which already have both indexes.troubleshooting/database-errors.mdwith the manual SQL workaround, plus an AGENTS.md Database-conventions rule against index hints on indexes that upgraded installs may lack.Tests
web/tests/integration/BansCompositeIndexesTest.php:PruneBans()archives matching Steam / IP submissions with both indexes dropped. Onmainthis test fails with the exact 1176 error from the issue.struc.sqlcolumn order.tearDownrestores the schema.Local gates: PHPStan is clean. PHPUnit passes apart from the six
PluginVersionResolveTestcases, which fail only because the dev container doesn't mountgame/; they're unrelated and pass in CI.UpdaterMigrationPortableSqlTestpasses.An adversarial review pass found no blockers. Its findings are addressed in the second commit: the corrected list of affected surfaces, a single combined
ALTERso old MyISAM tables are rebuilt once, thetype_iperror variant in the docs, and a tighter rationale for the existing:prefix_commsFORCE INDEX (created)hints.