diff --git a/AGENTS.md b/AGENTS.md index eb50e8c06..32e95f4c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -496,6 +496,21 @@ 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 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 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`. - 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..24c7ada3b 100644 --- a/docs/src/content/docs/troubleshooting/database-errors.md +++ b/docs/src/content/docs/troubleshooting/database-errors.md @@ -122,6 +122,27 @@ 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'` 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 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 +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..1772f4bf6 --- /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..b0156e8f2 --- /dev/null +++ b/web/updater/data/811.php @@ -0,0 +1,58 @@ +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`)', +]; + +$addClauses = []; +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) { + $addClauses[] = "ADD INDEX `$name` $columns"; + } +} + +if ($addClauses !== []) { + // @phpstan-ignore variable.undefined + $this->dbs->query('ALTER TABLE `:prefix_bans` ' . implode(', ', $addClauses)); + // @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" }