diff --git a/doc/command-line-flags.md b/doc/command-line-flags.md index d017c7c25..6a494ebbe 100644 --- a/doc/command-line-flags.md +++ b/doc/command-line-flags.md @@ -14,6 +14,8 @@ Allows the user to make schema changes that include a zero date or zero in date Mandatory unless using [`--revert`](#revert). The schema change to apply to the migrated table. You can pass just the alter options, for example `--alter="ADD COLUMN created_at timestamp NULL"`, together with [`--database`](#database) and [`--table`](#table). You can also pass a full `ALTER TABLE [database.]table ...` statement; an explicit database or table in `--alter` can be used instead of the corresponding flag. +Do **not** end the statement with a semicolon (`;`). A trailing semicolon is rejected: it breaks `--attempt-instant-ddl` because `ALGORITHM=INSTANT` is appended after the statement, and it has caused empty/broken indexes in some MySQL/MariaDB edge cases. Prefer the options-only form for clarity. + ### azure Add this flag when executing on Azure Database for MySQL. diff --git a/go/cmd/gh-ost/main.go b/go/cmd/gh-ost/main.go index d77046231..11c1145f0 100644 --- a/go/cmd/gh-ost/main.go +++ b/go/cmd/gh-ost/main.go @@ -12,6 +12,7 @@ import ( "os" "os/signal" "regexp" + "strings" "syscall" "time" @@ -232,6 +233,29 @@ func main() { if migrationContext.AlterStatement == "" && !migrationContext.Revert { log.Fatal("--alter must be provided and statement must not be empty") } + // Validate --alter shape early. A trailing semicolon breaks --attempt-instant-ddl + // (ALGORITHM=INSTANT is appended after the ';' and MySQL rejects the statement). + // Prefer options-only form ("ADD COLUMN ..."); full ALTER TABLE is still accepted + // so --alter can supply database/table, but a leading ALTER TABLE with a trailing + // semicolon is a common footgun (see github/gh-ost#1431). + if migrationContext.AlterStatement != "" { + trimmedAlter := strings.TrimSpace(migrationContext.AlterStatement) + if strings.HasSuffix(trimmedAlter, ";") { + log.Fatal("--alter must not end with a semicolon ('; 'ADD COLUMN foo INT' not 'ADD COLUMN foo INT;')") + } + // Case-insensitive check for a full ALTER TABLE statement prefix. + lowerAlter := strings.ToLower(trimmedAlter) + if strings.HasPrefix(lowerAlter, "alter table") { + // Full form is supported for specifying schema/table, but reject the + // redundant case where users also pass --table/--database and paste a + // complete DDL that still ends with options after ALTER TABLE name — + // document the preferred options-only form via a clear fatal when the + // statement is ONLY "ALTER TABLE" with no options (empty after strip). + // Always warn: options-only is the recommended CLI form. + log.Warning("--alter includes 'ALTER TABLE ...'; preferred form is options only (e.g. --alter=\"ADD COLUMN ...\") with --database/--table. Full ALTER TABLE form is accepted for schema/table discovery.") + } + migrationContext.AlterStatement = trimmedAlter + } parser := sql.NewParserFromAlterStatement(migrationContext.AlterStatement) migrationContext.AlterStatementOptions = parser.GetAlterStatementOptions()