--alter is unneccessary

Author: meiji163

go/cmd/gh-ost/main.go

@@ -208,12 +208,35 @@ func main() {
 
 	migrationContext.SetConnectionCharset(*charset)
 
-	if migrationContext.AlterStatement == "" {
+	if migrationContext.AlterStatement == "" && !migrationContext.Revert {
 		log.Fatal("--alter must be provided and statement must not be empty")
 	}
 	parser := sql.NewParserFromAlterStatement(migrationContext.AlterStatement)
 	migrationContext.AlterStatementOptions = parser.GetAlterStatementOptions()
 
+	if migrationContext.Revert {
+		if migrationContext.Resume {
+			log.Fatal("--revert cannot be used with --resume")
+		}
+		if migrationContext.OldTableName == "" {
+			migrationContext.Log.Fatalf("--revert must be called with --old-table")
+		}
+
+		// options irrelevant to revert mode
+		if migrationContext.AlterStatement != "" {
+			log.Warning("--alter was provided with --revert, it will be ignored")
+		}
+		if migrationContext.AttemptInstantDDL {
+			log.Warning("--attempt-instant-ddl was provided with --revert, it will be ignored")
+		}
+		if migrationContext.IncludeTriggers {
+			log.Warning("--include-triggers was provided with --revert, it will be ignored")
+		}
+		if migrationContext.DiscardForeignKeys {
+			log.Warning("--discard-foreign-keys was provided with --revert, it will be ignored")
+		}
+	}
+
 	if migrationContext.DatabaseName == "" {
 		if parser.HasExplicitSchema() {
 			migrationContext.DatabaseName = parser.GetExplicitSchema()
@@ -293,10 +316,6 @@ func main() {
 		migrationContext.Log.Fatalf("--checkpoint-seconds should be >=10")
 	}
 
-	if migrationContext.Revert && migrationContext.OldTableName == "" {
-		migrationContext.Log.Fatalf("--revert must be called with --old-table")
-	}
-
 	switch *cutOver {
 	case "atomic", "default", "":
 		migrationContext.CutOverType = base.CutOverAtomic
@@ -353,15 +372,16 @@ func main() {
 	acceptSignals(migrationContext)
 
 	migrator := logic.NewMigrator(migrationContext, AppVersion)
+	var err error
 	if migrationContext.Revert {
-		if err := migrator.Revert(); err != nil {
-			migrationContext.Log.Fatale(err)
-		}
+		err = migrator.Revert()
 	} else {
-		if err := migrator.Migrate(); err != nil {
-			migrator.ExecOnFailureHook()
-			migrationContext.Log.Fatale(err)
-		}
+		err = migrator.Migrate()
+	}
+
+	if err != nil {
+		migrator.ExecOnFailureHook()
+		migrationContext.Log.Fatale(err)
 	}
 	fmt.Fprintln(os.Stdout, "# Done")
 }

go/logic/migrator.go

@@ -541,9 +541,6 @@ func (this *Migrator) Revert() error {
 	if err := this.hooksExecutor.onStartup(); err != nil {
 		return err
 	}
-	if err := this.parser.ParseAlterStatement(this.migrationContext.AlterStatement); err != nil {
-		return err
-	}
 	if err := this.validateAlterStatement(); err != nil {
 		return err
 	}
@@ -591,7 +588,10 @@ func (this *Migrator) Revert() error {
 	}
 
 	this.initiateThrottler()
+	go this.initiateStatus()
 	go this.executeDMLWriteFuncs()
+
+	this.printStatus(ForcePrintStatusRule)
 	var retrier func(func() error, ...bool) error
 	if this.migrationContext.CutOverExponentialBackoff {
 		retrier = this.retryOperationWithExponentialBackoff
@@ -605,6 +605,9 @@ func (this *Migrator) Revert() error {
 		return err
 	}
 	atomic.StoreInt64(&this.migrationContext.CutOverCompleteFlag, 1)
+	if err := this.finalCleanup(); err != nil {
+		return nil
+	}
 	if err := this.hooksExecutor.onSuccess(); err != nil {
 		return err
 	}

go/logic/migrator_test.go

@@ -725,7 +725,6 @@ func (suite *MigratorTestSuite) TestRevert() {
 		migrationContext.ApplierConnectionConfig = connectionConfig
 		migrationContext.InspectorConnectionConfig = connectionConfig
 		migrationContext.SetConnectionConfig("innodb")
-		migrationContext.AlterStatement = "DROP INDEX idx1"
 		migrationContext.DropServeSocket = true
 		migrationContext.UseGTIDs = true
 		migrationContext.Revert = true