Repository navigation
fix(updater): portable information_schema guard for lockout columns (#1498) - #1499
Merged
Merged
Conversation
…1498) Migration 801.php added the admin lockout columns with `ALTER TABLE ... ADD IF NOT EXISTS`, a MariaDB-only DDL extension. Stock MySQL (8.x included) and MySQL-compatible engines like Percona Server reject it with SQLSTATE[42000] 1064 mid-upgrade, so panels on those engines could not upgrade past 801 (v2 rc5 -> rc6, v1.8.x -> v2). The dev stack and CI both run MariaDB, which accepts the syntax, so the runtime idempotency test and PHPStan's dba gate both passed it through; the break only surfaced on self-hosters running MySQL / Percona, both first-class supported engines per the docs. Guard each ADD with a portable information_schema existence probe + a plain `ADD COLUMN`. Same idempotent contract, runs on both engines, converges to the same schema. Edited 801.php in place rather than shipping a new migration because the effect is unchanged (the Updater only runs versions above config.version, so MariaDB installs that already ran 801 never re-run it; MySQL installs that fataled never advanced past it and pick up the fixed version on the next pass). Add UpdaterMigrationPortableSqlTest, a static source-scan guard that fails if any v2-era migration (version >= 800) reuses the MariaDB-only `ADD ... IF NOT EXISTS` form. A runtime test can't catch this (the suite runs against MariaDB). Ten pre-800 migrations carry the same syntax but only run on ancient-install -> v2 paths; they're a tracked follow-up, deliberately out of scope here.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Fixes #1498. Migration
web/updater/data/801.phpadded the admin lockout columns withALTER TABLE ... ADD IF NOT EXISTS, a MariaDB-only DDL extension. Stock MySQL (8.x included) and MySQL-compatible engines such as Percona Server reject it withSQLSTATE[42000] 1064mid-upgrade, so panels on those engines could not upgrade past 801 (the blocker on every supported path: v2 rc5 -> rc6 and v1.8.x -> v2 both start atconfig.version = 705).Root cause
ADD IF NOT EXISTSwas introduced in #1473 to make #1472's fix idempotent (panels that already carry the columns must not fatal withSQLSTATE[42S21]"Duplicate column name"). It does that on MariaDB, but the syntax is non-portable. Because the dev stack and CI both run MariaDB (which accepts it), the runtime idempotency test (Updater801LockoutColumnsTest) and PHPStan'sdbagate (which also introspects MariaDB) both passed it through. The break only surfaced on self-hosters running MySQL / Percona, both first-class supported engines per the docs (prerequisites.mdx: "MySQL >= 5.6 ... 8.0+ is fine and recommended").Fix
801.phpto guard eachADDwith a portableinformation_schema.COLUMNSexistence probe + a plainADD COLUMN. Same idempotent contract, runs on both engines, converges to the same schema.801.phpin place rather than shipping a new migration: the effect is unchanged. The Updater only runs versions aboveconfig.version, so MariaDB installs that already ran 801 never re-run it; MySQL/Percona installs that fataled never advanced past it and pick up the fixed version on the next pass. Fresh installs get the columns fromstruc.sqland never run the updater. (Per the AGENTS.md "Updater migrations" rule, in-place edits are correct when the script's effect doesn't change.)$this->dbsreads (supplied byUpdater::update()'s instance scope) are suppressed inline with@phpstan-ignore variable.undefined; the stalephpstan-baseline.neonentry for801.phpis removed.Regression guard
Adds
web/tests/integration/UpdaterMigrationPortableSqlTest.php— a static source-scan guard (mirrorsDeadJsCallSitesTest's pure-file-scan shape) that fails if any v2-era migration (version >= 800) reuses the MariaDB-onlyADD ... IF NOT EXISTSform. A runtime test can't catch this class of bug because the suite runs against MariaDB.CREATE TABLE IF NOT EXISTS(valid on every engine) is intentionally not matched. Comments are stripped viaphp_strip_whitespace()so 801's own explanatory docblock doesn't false-fire.Out of scope / follow-up
Ten pre-800 migrations carry the same MariaDB-only syntax (1, 112, 150, 153, 160, 241, 291, 295, 351, 355). They only run on ancient (pre-356, SB 1.5.x-era) install -> v2 paths, a real but rarer scenario, and a couple have quirks (295 is a compound
ADD ..., ADD ...; 355 hardcodes ansb_modsprefix). Deliberately left out so this fix stays small and low-risk; tracked as a follow-up. The new test's>= 800floor guards every migration added going forward.Test plan
./sbpp.sh test --filter='Updater801LockoutColumns|UpdaterMigrationPortableSql'— 3 tests, 23 assertions, green../sbpp.sh phpstan— no errors (validates the inline ignores, the baseline removal, the new test file, and that theinformation_schemaSELECT passes thedbagate).config.version = 705completes past 801 without the 1064 fatal.