From 1317cd8123ff8bc4c0b3600e7d72f09f48e594c7 Mon Sep 17 00:00:00 2001 From: Fred Date: Wed, 28 Oct 2026 18:45:06 +0100 Subject: [PATCH] Schema updater: repair and retry once when a step names a missing table A 1.5.10.2518 server arrives at vValue 288. 1.6 fills positions 287 and 288 with CREATE TABLE fileDeleteQueue and its first ALTERs, so an upgrade skips them and step 290 ALTERs a table that is not there: 1146. That code is not tolerated, so the update stopped. The reconcile after the loop then created the table, but the installer had already printed "Failed!" with the server left at 289, and only a second run finished the upgrade. On 1146, run SchemaReconciler::reconcile() at the point of failure and execute the statement once more. That is the second run's path, inside the first. Any other error, a repair that fails, or a retry that still fails is reported exactly as before. Measured on a fresh 1.5.10.2518 (stable c97e20500) upgraded to rc-1.6.0 at 4983c250f: one pass, Setup complete, vValue 445, no fog_schema_update_error.log, and an image captured on 1.5 deploys from the upgraded server. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/web/src/Pages/SchemaUpdaterPage.php | 45 +++++++ ...ema-updater-repairs-missing-table.test.php | 118 ++++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 tests/schema-updater-repairs-missing-table.test.php diff --git a/packages/web/src/Pages/SchemaUpdaterPage.php b/packages/web/src/Pages/SchemaUpdaterPage.php index f8d2fec880..3541f3a33c 100644 --- a/packages/web/src/Pages/SchemaUpdaterPage.php +++ b/packages/web/src/Pages/SchemaUpdaterPage.php @@ -402,6 +402,9 @@ public function indexPost() if (in_array($err, $skiperrs)) { continue; } + if (self::_repairAndRetry($update, $err, $skiperrs)) { + continue; + } $errors[] = sprintf( "%s: %s\n", _('Update ID'), @@ -579,6 +582,48 @@ public function indexPost() } $this->jsonSend($code, $msg); } + /** + * Repair the structure and run the statement once more after a 1146. + * + * 1146 means the step names a table this database never got. Steps are + * positions, not identities, and 1.5.x fills the same positions with + * different migrations: 1.5.10.2518 arrives at 288, so 1.6's + * `CREATE TABLE fileDeleteQueue` (`// 287`) is skipped and `// 290` + * ALTERs a table that is not there. The reconcile after the loop creates + * it, but only once the update has already failed, so the installer + * printed "Failed!" and a second run was needed. This runs that repair at + * the point of failure instead. + * + * Any other error is left to the caller. So is a repair that does not + * succeed, and a retry that still fails: one repair, one retry. + * + * @param string $update the statement that failed + * @param int|string $err its MySQL error number + * @param array $skiperrs the codes the updater tolerates + * @param callable|null $repair SchemaReconciler::reconcile() unless given + * + * @return bool true when the statement now stands + */ + private static function _repairAndRetry( + $update, + $err, + array $skiperrs, + $repair = null + ) { + if (1146 != $err) { + return false; + } + if (true !== call_user_func($repair ?: [SchemaReconciler::class, 'reconcile'])) { + // Leave the statement's own error behind for the caller to + // report, not whatever the repair ran last. + self::$DB->query($update); + return false; + } + if (false === self::$DB->query($update)->error) { + return true; + } + return in_array(self::$DB->errorCode, $skiperrs); + } /** * Drop NO_ZERO_DATE and NO_ZERO_IN_DATE from this session's sql_mode. * diff --git a/tests/schema-updater-repairs-missing-table.test.php b/tests/schema-updater-repairs-missing-table.test.php new file mode 100644 index 0000000000..b0e45491f4 --- /dev/null +++ b/tests/schema-updater-repairs-missing-table.test.php @@ -0,0 +1,118 @@ +setAccessible(true); + +$skiperrs = [1050, 1054, 1060, 1061, 1062, 1091]; +$alter = 'ALTER TABLE `fileDeleteQueue` ADD `fdqPathType` VARCHAR(255) NOT NULL'; + +/** + * Counters the closures below write into. An object rather than by-reference + * ints, so each check reads what the code under test actually did. + */ +$seen = new \stdClass(); + +/** + * Answer $alter with $codes in turn (0 = success) and everything else with + * success. Resets the counters. + */ +$serve = static function (array $codes) use ($db, $alter, $seen) { + $seen->runs = 0; + $seen->repairs = 0; + $db->responder = static function ($sql) use ($db, $alter, &$codes, $seen) { + $code = 0; + if ($sql === $alter) { + $seen->runs++; + $code = $codes ? array_shift($codes) : 0; + } + $db->error = $code ? "error $code" : false; + $db->errorCode = $code ?: false; + return []; + }; +}; + +$repairOk = static function () use ($seen) { + $seen->repairs++; + return true; +}; + +// --- 1146, the repair creates the table, the retry succeeds ----------------- +$serve([0]); +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairOk); +$t->check('1146: reported as handled', $ok === true); +$t->check('1146: the repair ran once', $seen->repairs === 1); +$t->check('1146: the statement ran again', $seen->runs === 1); + +// --- 1146, the retry hits a tolerated code (the repair added the column) ---- +$serve([1060]); +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairOk); +$t->check('1146 then 1060: handled like any tolerated duplicate', $ok === true); + +// --- 1146, the retry still fails -------------------------------------------- +$serve([1146]); +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairOk); +$t->check('1146 twice: left to the caller', $ok === false); +$t->check('1146 twice: retried once, not looped', $seen->runs === 1 && $seen->repairs === 1); +$t->check('1146 twice: the caller sees the statement\'s own error', $db->errorCode === 1146); + +// --- 1146, the repair itself fails ------------------------------------------ +$serve([1146]); +$repairFails = static function () use ($seen) { + $seen->repairs++; + return 'reconcile failed'; +}; +$ok = $retry->invoke(null, $alter, 1146, $skiperrs, $repairFails); +$t->check('failed repair: left to the caller', $ok === false); +$t->check('failed repair: the caller sees the statement\'s own error', $db->errorCode === 1146); + +// --- any other error is not this method's business -------------------------- +foreach ([1064, 1452, 1005] as $code) { + $serve([]); + $ok = $retry->invoke(null, $alter, $code, $skiperrs, $repairOk); + $t->check("$code: left to the caller", $ok === false); + $t->check("$code: no repair, no retry", $seen->repairs === 0 && $seen->runs === 0); +} + +// --- the production call site ------------------------------------------------ +$src = file_get_contents(dirname(__DIR__) . '/packages/web/src/Pages/SchemaUpdaterPage.php'); +$t->check( + 'update() consults _repairAndRetry() after the tolerated codes', + (bool)preg_match( + '/in_array\(\$err, \$skiperrs\)\)\s*\{\s*continue;\s*\}\s*if \(self::_repairAndRetry\(\$update, \$err, \$skiperrs\)\)\s*\{\s*continue;\s*\}/', + $src + ) +); + +$t->finish();