fix(db): don't hint missing ban indexes in PruneBans (#1578)

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`

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 …`

web/includes/system-functions.php

@@ -331,19 +331,21 @@ 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
               WHERE S.`archiv` = 0
               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

web/tests/integration/BansCompositeIndexesTest.php

@@ -0,0 +1,160 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sbpp\Tests\Integration;
+
+use PDO;
+use Sbpp\Tests\ApiTestCase;
+use Sbpp\Tests\Fixture;
+
+/**
+ * #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 and the add / edit ban flows.
+ *
+ * Surfaces exercised:
+ *   - `PruneBans()` runs and archives matching submissions with both
+ *     indexes dropped.
+ *   - `web/updater/data/811.php` recreates missing indexes with the
+ *     `struc.sql` column order, including the one-of-two partial case.
+ *   - Re-running the migration is a no-op (no duplicate indexes).
+ *
+ * DDL survives `Fixture::truncateAndReseed`, so `tearDown` restores
+ * both indexes to keep sibling tests on the fresh-install schema.
+ */
+final class BansCompositeIndexesTest extends ApiTestCase
+{
+    private const INDEXES = [
+        'type_authid' => ['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<string>
+     */
+    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());
+    }
+}

web/updater/data/811.php

@@ -0,0 +1,58 @@
+<?php
+
+// Issue #1578: `:prefix_bans` ships `KEY type_authid (type, authid)` and
+// `KEY type_ip (type, ip)` in `struc.sql`, and `702.php` adds them for
+// installs that upgraded through that range. Installs that reached a
+// 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 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. 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
+// `$this->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;

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"
 }