From 452193063cab4b618bc27240b60f271899ca0d06 Mon Sep 17 00:00:00 2001 From: rumblefrog Date: Mon, 28 Sep 2026 16:58:15 -0400 Subject: [PATCH 1/2] fix(db): don't hint missing ban indexes in PruneBans (#1578) 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. --- AGENTS.md | 13 ++ .../docs/troubleshooting/database-errors.md | 20 +++ web/includes/system-functions.php | 6 +- .../integration/BansCompositeIndexesTest.php | 160 ++++++++++++++++++ web/updater/data/811.php | 53 ++++++ web/updater/store.json | 3 +- 6 files changed, 252 insertions(+), 3 deletions(-) create mode 100644 web/tests/integration/BansCompositeIndexesTest.php create mode 100644 web/updater/data/811.php diff --git a/AGENTS.md b/AGENTS.md index eb50e8c06..3a06d21fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -496,6 +496,19 @@ top-level `class Foo {}` in `web/includes/` (see "Anti-patterns"). `atomic: true` to `executeInList()` when splitting a formerly single write must preserve all-or-nothing behavior; the helper owns the transaction in that mode, so callers must not open a nested one. +- Don't add `FORCE INDEX` / `USE INDEX` hints naming an index that + only `struc.sql` or a single `ALTER TABLE … ADD INDEX` migration + creates. The updater only runs scripts above the stored + `config.version`, so a long-upgraded install whose version moved + past that script without running it never gets the index, and a + hint on a missing index is MariaDB error 1176, not a no-op. (The + `:prefix_comms` `FORCE INDEX (created)` hints are fine: that index + ships in the table's own `CREATE TABLE`.) #1578: 2.2.1's + `PruneBans()` hinted `type_authid` / `type_ip` and fataled the + banlist, servers, and dashboard on such installs. If a hint is + truly needed, ship a paired idempotent migration that creates the + index (see `web/updater/data/811.php`) in the same PR. Regression + guard: `web/tests/integration/BansCompositeIndexesTest.php`. - Each named placeholder (`:name`) inside one query needs as many `bind()` calls as occurrences. The panel runs PDO with `PDO::ATTR_EMULATE_PREPARES => false` (`Sbpp\Db\Database::__construct` diff --git a/docs/src/content/docs/troubleshooting/database-errors.md b/docs/src/content/docs/troubleshooting/database-errors.md index 6b70e5757..b1e3faabd 100644 --- a/docs/src/content/docs/troubleshooting/database-errors.md +++ b/docs/src/content/docs/troubleshooting/database-errors.md @@ -122,6 +122,26 @@ Watch the output for errors. If a specific migration step fails, the page will say which one and why. Share that in `#help-support` if you can't decipher it. +## Key doesn't exist + +> `1176 Key 'type_authid' doesn't exist in table 'BSteam'` + +Panel 2.2.1 on an older, long-upgraded install can hit this on the +Ban List, Servers, and Home pages. Your `_bans` table is missing two +indexes the panel expects. + +Upgrade to the next panel release and run the updater. It adds the +missing indexes. To fix it right away instead, run this against the +panel database (swap `sb_` for your table prefix): + +```sql +ALTER TABLE `sb_bans` ADD INDEX `type_authid` (`type`, `authid`); +ALTER TABLE `sb_bans` ADD INDEX `type_ip` (`type`, `ip`); +``` + +If one statement fails with `Duplicate key name`, that index already +exists. Skip it. + ## Incorrect string value > `Incorrect string value: '\xF0\x9F…' for column …` diff --git a/web/includes/system-functions.php b/web/includes/system-functions.php index 687dfa189..6b7636708 100644 --- a/web/includes/system-functions.php +++ b/web/includes/system-functions.php @@ -331,11 +331,13 @@ function PruneBans(): void // that happens to match both its Steam ID and IP. Keeping the arms // separate lets MariaDB probe type_authid / type_ip directly for each // submission instead of materialising all active identifiers first. + // No index hints: upgraded installs can lack those indexes (#1578), + // and FORCE/USE INDEX on a missing index is error 1176, not a no-op. $subIds = $pdo ->query( 'SELECT S.`subid` FROM `:prefix_submissions` AS S - INNER JOIN `:prefix_bans` AS BSteam FORCE INDEX (`type_authid`) + INNER JOIN `:prefix_bans` AS BSteam ON BSteam.`type` = 0 AND BSteam.`authid` = S.`SteamId` AND BSteam.`RemoveType` IS NULL @@ -343,7 +345,7 @@ function PruneBans(): void UNION DISTINCT SELECT S.`subid` FROM `:prefix_submissions` AS S - INNER JOIN `:prefix_bans` AS BIp FORCE INDEX (`type_ip`) + INNER JOIN `:prefix_bans` AS BIp ON BIp.`type` = 1 AND BIp.`ip` = S.`sip` AND BIp.`RemoveType` IS NULL diff --git a/web/tests/integration/BansCompositeIndexesTest.php b/web/tests/integration/BansCompositeIndexesTest.php new file mode 100644 index 000000000..641aaacbf --- /dev/null +++ b/web/tests/integration/BansCompositeIndexesTest.php @@ -0,0 +1,160 @@ + ['type', 'authid'], + 'type_ip' => ['type', 'ip'], + ]; + + protected function tearDown(): void + { + $pdo = Fixture::rawPdo(); + foreach (self::INDEXES as $name => $columns) { + if ($this->indexColumns($name) === []) { + $pdo->exec(sprintf( + 'ALTER TABLE `%s_bans` ADD INDEX `%s` (`%s`)', + DB_PREFIX, + $name, + implode('`, `', $columns), + )); + } + } + parent::tearDown(); + } + + private function runMigration(): bool + { + $ctx = new class($GLOBALS['PDO']) { + public function __construct(public \Database $dbs) {} + public function run(string $path): mixed { return require $path; } + }; + return (bool) $ctx->run(ROOT . 'updater/data/811.php'); + } + + private function dropIndex(string $name): void + { + Fixture::rawPdo()->exec(sprintf('ALTER TABLE `%s_bans` DROP INDEX `%s`', DB_PREFIX, $name)); + } + + /** + * @return list + */ + private function indexColumns(string $name): array + { + $stmt = Fixture::rawPdo()->prepare( + 'SELECT COLUMN_NAME FROM information_schema.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = ? AND INDEX_NAME = ? + ORDER BY SEQ_IN_INDEX' + ); + $stmt->execute([DB_PREFIX . '_bans', $name]); + return array_map('strval', $stmt->fetchAll(PDO::FETCH_COLUMN)); + } + + private function bansIndexCount(): int + { + $stmt = Fixture::rawPdo()->prepare( + 'SELECT COUNT(DISTINCT INDEX_NAME) FROM information_schema.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = ?' + ); + $stmt->execute([DB_PREFIX . '_bans']); + return (int) $stmt->fetchColumn(); + } + + public function testPruneBansRunsWithoutCompositeIndexes(): void + { + $this->dropIndex('type_authid'); + $this->dropIndex('type_ip'); + + $pdo = Fixture::rawPdo(); + $now = time(); + $pdo->prepare(sprintf( + 'INSERT INTO `%s_bans` + (type, ip, authid, name, created, ends, length, reason, aid, adminIp, sid) + VALUES (?, ?, ?, ?, ?, 0, 0, "test", ?, "127.0.0.1", 0)', + DB_PREFIX, + ))->execute([0, null, 'STEAM_0:1:157801', 'active-steam', $now, Fixture::adminAid()]); + $pdo->prepare(sprintf( + 'INSERT INTO `%s_bans` + (type, ip, authid, name, created, ends, length, reason, aid, adminIp, sid) + VALUES (?, ?, ?, ?, ?, 0, 0, "test", ?, "127.0.0.1", 0)', + DB_PREFIX, + ))->execute([1, '203.0.113.78', '', 'active-ip', $now, Fixture::adminAid()]); + + $insertSubmission = $pdo->prepare(sprintf( + 'INSERT INTO `%s_submissions` + (submitted, ModID, SteamId, name, email, reason, ip, sip, archiv) + VALUES (?, 0, ?, ?, "player@example.com", "test", "127.0.0.1", ?, 0)', + DB_PREFIX, + )); + $insertSubmission->execute([$now, 'STEAM_0:1:157801', 'steam-match', null]); + $insertSubmission->execute([$now, '', 'ip-match', '203.0.113.78']); + $insertSubmission->execute([$now, 'STEAM_0:1:157899', 'unmatched', null]); + + \PruneBans(); + + $rows = $pdo->query(sprintf('SELECT name, archiv FROM `%s_submissions`', DB_PREFIX)) + ->fetchAll(PDO::FETCH_KEY_PAIR); + $this->assertSame(3, (int) $rows['steam-match']); + $this->assertSame(3, (int) $rows['ip-match']); + $this->assertSame(0, (int) $rows['unmatched']); + } + + public function testMigrationRecreatesMissingIndexes(): void + { + $this->dropIndex('type_authid'); + $this->dropIndex('type_ip'); + + $this->assertTrue($this->runMigration()); + + foreach (self::INDEXES as $name => $columns) { + $this->assertSame($columns, $this->indexColumns($name), "$name must match struc.sql"); + } + } + + public function testMigrationAddsOnlyTheMissingIndex(): void + { + $this->dropIndex('type_ip'); + $before = $this->bansIndexCount(); + + $this->assertTrue($this->runMigration()); + + $this->assertSame(['type', 'authid'], $this->indexColumns('type_authid')); + $this->assertSame(['type', 'ip'], $this->indexColumns('type_ip')); + $this->assertSame($before + 1, $this->bansIndexCount()); + } + + public function testMigrationIsNoOpWhenIndexesExist(): void + { + $before = $this->bansIndexCount(); + + $this->assertTrue($this->runMigration()); + $this->assertTrue($this->runMigration()); + + $this->assertSame($before, $this->bansIndexCount()); + } +} diff --git a/web/updater/data/811.php b/web/updater/data/811.php new file mode 100644 index 000000000..7a0ec055b --- /dev/null +++ b/web/updater/data/811.php @@ -0,0 +1,53 @@ +dbs` call is suppressed in the same way as sibling migrations. + +// @phpstan-ignore variable.undefined +$bansTable = $this->dbs->getPrefix() . '_bans'; + +$indexes = [ + 'type_authid' => '(`type`, `authid`)', + 'type_ip' => '(`type`, `ip`)', +]; + +foreach ($indexes as $name => $columns) { + // @phpstan-ignore variable.undefined + $this->dbs->query( + 'SELECT COUNT(*) AS n FROM information_schema.STATISTICS' + . ' WHERE TABLE_SCHEMA = DATABASE()' + . ' AND TABLE_NAME = :table' + . ' AND INDEX_NAME = :index' + ); + // @phpstan-ignore variable.undefined + $this->dbs->bind(':table', $bansTable); + // @phpstan-ignore variable.undefined + $this->dbs->bind(':index', $name); + // @phpstan-ignore variable.undefined + $row = $this->dbs->single(); + + if ((int) ($row['n'] ?? 0) > 0) { + continue; + } + + // @phpstan-ignore variable.undefined + $this->dbs->query("ALTER TABLE `:prefix_bans` ADD INDEX `$name` $columns"); + // @phpstan-ignore variable.undefined + $this->dbs->execute(); +} + +return true; diff --git a/web/updater/store.json b/web/updater/store.json index 10e643d19..0378c8f28 100644 --- a/web/updater/store.json +++ b/web/updater/store.json @@ -48,5 +48,6 @@ "807": "807.php", "808": "808.php", "809": "809.php", - "810": "810.php" + "810": "810.php", + "811": "811.php" } From a19c71f35ad30bd104077811233e81f84a688c54 Mon Sep 17 00:00:00 2001 From: rumblefrog Date: Mon, 28 Sep 2026 17:02:56 -0400 Subject: [PATCH 2/2] fix(db): address review for #1578 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. --- AGENTS.md | 8 +++++--- .../docs/troubleshooting/database-errors.md | 7 ++++--- .../integration/BansCompositeIndexesTest.php | 2 +- web/updater/data/811.php | 17 +++++++++++------ 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3a06d21fb..32e95f4c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -502,10 +502,12 @@ top-level `class Foo {}` in `web/includes/` (see "Anti-patterns"). `config.version`, so a long-upgraded install whose version moved past that script without running it never gets the index, and a hint on a missing index is MariaDB error 1176, not a no-op. (The - `:prefix_comms` `FORCE INDEX (created)` hints are fine: that index - ships in the table's own `CREATE TABLE`.) #1578: 2.2.1's + `:prefix_comms` `FORCE INDEX (created)` hints in `page.commslist.php` + are the accepted exception: `KEY created` is in every schema that + table has shipped with, upstream SourceComms included, and the hint + has been live since 1.5.0F without reports.) #1578: 2.2.1's `PruneBans()` hinted `type_authid` / `type_ip` and fataled the - banlist, servers, and dashboard on such installs. If a hint is + banlist plus the add / edit ban flows on such installs. If a hint is truly needed, ship a paired idempotent migration that creates the index (see `web/updater/data/811.php`) in the same PR. Regression guard: `web/tests/integration/BansCompositeIndexesTest.php`. diff --git a/docs/src/content/docs/troubleshooting/database-errors.md b/docs/src/content/docs/troubleshooting/database-errors.md index b1e3faabd..24c7ada3b 100644 --- a/docs/src/content/docs/troubleshooting/database-errors.md +++ b/docs/src/content/docs/troubleshooting/database-errors.md @@ -124,11 +124,12 @@ if you can't decipher it. ## Key doesn't exist -> `1176 Key 'type_authid' doesn't exist in table 'BSteam'` +> `1176 Key 'type_authid' doesn't exist in table 'BSteam'` or +> `1176 Key 'type_ip' doesn't exist in table 'BIp'` Panel 2.2.1 on an older, long-upgraded install can hit this on the -Ban List, Servers, and Home pages. Your `_bans` table is missing two -indexes the panel expects. +Ban List and when adding or editing a ban. Your `_bans` table is +missing two indexes the panel expects. Upgrade to the next panel release and run the updater. It adds the missing indexes. To fix it right away instead, run this against the diff --git a/web/tests/integration/BansCompositeIndexesTest.php b/web/tests/integration/BansCompositeIndexesTest.php index 641aaacbf..1772f4bf6 100644 --- a/web/tests/integration/BansCompositeIndexesTest.php +++ b/web/tests/integration/BansCompositeIndexesTest.php @@ -12,7 +12,7 @@ * #1578 — upgraded installs can lack `:prefix_bans`'s `type_authid` / * `type_ip` composite indexes. 2.2.1's `PruneBans()` named them in * `FORCE INDEX` hints, which MariaDB rejects with error 1176 when the - * index is missing, fataling the banlist / servers / dashboard pages. + * index is missing, fataling the banlist and the add / edit ban flows. * * Surfaces exercised: * - `PruneBans()` runs and archives matching submissions with both diff --git a/web/updater/data/811.php b/web/updater/data/811.php index 7a0ec055b..b0156e8f2 100644 --- a/web/updater/data/811.php +++ b/web/updater/data/811.php @@ -6,12 +6,14 @@ // later `config.version` by another path (old installer seeds, manual // schema imports, forks) never got them, so `PruneBans()`'s submission // lookup had no composite index to probe and 2.2.1's `FORCE INDEX` -// hints fataled every banlist / servers / dashboard render. +// hints fataled the banlist and the add / edit ban flows. // // Add each index only when no index of that name exists. The // `information_schema` probe keeps this portable (MySQL has no -// `ADD INDEX IF NOT EXISTS`) and makes re-runs a no-op. Fresh installs -// already carry both indexes from `struc.sql`, so they converge. +// `ADD INDEX IF NOT EXISTS`) and makes re-runs a no-op. Missing indexes +// go in one `ALTER TABLE` so old MyISAM tables are rebuilt once, not +// twice. Fresh installs already carry both indexes from `struc.sql`, so +// they converge. // // `$this` is supplied by Updater::update() which loads this file inside // the Updater instance scope; PHPStan can't see that, so each @@ -25,6 +27,7 @@ 'type_ip' => '(`type`, `ip`)', ]; +$addClauses = []; foreach ($indexes as $name => $columns) { // @phpstan-ignore variable.undefined $this->dbs->query( @@ -40,12 +43,14 @@ // @phpstan-ignore variable.undefined $row = $this->dbs->single(); - if ((int) ($row['n'] ?? 0) > 0) { - continue; + if ((int) ($row['n'] ?? 0) === 0) { + $addClauses[] = "ADD INDEX `$name` $columns"; } +} +if ($addClauses !== []) { // @phpstan-ignore variable.undefined - $this->dbs->query("ALTER TABLE `:prefix_bans` ADD INDEX `$name` $columns"); + $this->dbs->query('ALTER TABLE `:prefix_bans` ' . implode(', ', $addClauses)); // @phpstan-ignore variable.undefined $this->dbs->execute(); }