Online DDL: remove legacy (and ancient) 'REVERT <uuid>' syntax

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

go/vt/schema/online_ddl.go

@@ -223,35 +223,21 @@ func NewOnlineDDL(keyspace string, table string, sql string, ddlStrategySetting
 				encodeDirective(string(ddlStrategySetting.Strategy)),
 				encodeDirective(ddlStrategySetting.Options),
 			)}
-		if uuid, err := legacyParseRevertUUID(sql); err == nil {
-			sql = fmt.Sprintf("revert vitess_migration '%s'", uuid)
-		}
 
 		stmt, err := parser.Parse(sql)
 		if err != nil {
-			isLegacyRevertStatement := false
-			// query validation and rebuilding
-			if _, err := legacyParseRevertUUID(sql); err == nil {
-				// This is a revert statement of the form "revert <uuid>". We allow this for now. Future work will
-				// make sure the statement is a valid, parseable "revert vitess_migration '<uuid>'", but we must
-				// be backwards compatible for now.
-				isLegacyRevertStatement = true
-			}
-			if !isLegacyRevertStatement {
-				// otherwise the statement should have been parseable!
-				return nil, err
-			}
-		} else {
-			switch stmt := stmt.(type) {
-			case sqlparser.DDLStatement:
-				stmt.SetComments(comments)
-			case *sqlparser.RevertMigration:
-				stmt.SetComments(comments)
-			default:
-				return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Unsupported statement for Online DDL: %v", sqlparser.String(stmt))
-			}
-			sql = sqlparser.String(stmt)
+			return nil, err
+		}
+		switch stmt := stmt.(type) {
+		case sqlparser.DDLStatement:
+			stmt.SetComments(comments)
+		case *sqlparser.RevertMigration:
+			stmt.SetComments(comments)
+		default:
+			return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Unsupported statement for Online DDL: %v", sqlparser.String(stmt))
 		}
+		sql = sqlparser.String(stmt)
+
 	}
 
 	return &OnlineDDL{
@@ -354,14 +340,7 @@ func (onlineDDL *OnlineDDL) sqlWithoutComments(parser *sqlparser.Parser) (sql st
 	sql = onlineDDL.SQL
 	stmt, err := parser.Parse(sql)
 	if err != nil {
-		// query validation and rebuilding
-		if _, err := legacyParseRevertUUID(sql); err == nil {
-			// This is a revert statement of the form "revert <uuid>". We allow this for now. Future work will
-			// make sure the statement is a valid, parseable "revert vitess_migration '<uuid>'", but we must
-			// be backwards compatible for now.
-			return sql, nil
-		}
-		// otherwise the statement should have been parseable!
+		// The statement should have been parseable!
 		return "", err
 	}
 
@@ -421,9 +400,6 @@ func (onlineDDL *OnlineDDL) GetActionStr(parser *sqlparser.Parser) (action sqlpa
 // fo the reverted migration.
 // The function returns error when this is not a revert migration.
 func (onlineDDL *OnlineDDL) GetRevertUUID(parser *sqlparser.Parser) (uuid string, err error) {
-	if uuid, err := legacyParseRevertUUID(onlineDDL.SQL); err == nil {
-		return uuid, nil
-	}
 	if stmt, err := parser.Parse(onlineDDL.SQL); err == nil {
 		if revert, ok := stmt.(*sqlparser.RevertMigration); ok {
 			return revert.UUID, nil

go/vt/schema/parser.go

@@ -17,7 +17,6 @@ limitations under the License.
 package schema
 
 import (
-	"fmt"
 	"regexp"
 	"strings"
 
@@ -50,7 +49,6 @@ var (
 		// ALTER TABLE tbl something
 		regexp.MustCompile(alterTableBasicPattern + `([\S]+)\s+(.*$)`),
 	}
-	revertStatementRegexp = regexp.MustCompile(`(?i)^revert\s+([\S]*)$`)
 
 	enumValuesRegexp = regexp.MustCompile("(?i)^enum[(](.*)[)]$")
 	setValuesRegexp  = regexp.MustCompile("(?i)^set[(](.*)[)]$")
@@ -79,19 +77,6 @@ func ParseAlterTableOptions(alterStatement string) (explicitSchema, explicitTabl
 	return explicitSchema, explicitTable, alterOptions
 }
 
-// legacyParseRevertUUID expects a query like "revert 4e5dcf80_354b_11eb_82cd_f875a4d24e90" and returns the UUID value.
-func legacyParseRevertUUID(sql string) (uuid string, err error) {
-	submatch := revertStatementRegexp.FindStringSubmatch(sql)
-	if len(submatch) == 0 {
-		return "", fmt.Errorf("Not a Revert DDL: '%s'", sql)
-	}
-	uuid = submatch[1]
-	if !IsOnlineDDLUUID(uuid) {
-		return "", fmt.Errorf("Not an online DDL UUID: '%s'", uuid)
-	}
-	return uuid, nil
-}
-
 // ParseEnumValues parses the comma delimited part of an enum column definition
 func ParseEnumValues(enumColumnType string) string {
 	if submatch := enumValuesRegexp.FindStringSubmatch(enumColumnType); len(submatch) > 0 {

go/vt/schema/parser_test.go

@@ -48,23 +48,6 @@ func TestParseAlterTableOptions(t *testing.T) {
 	}
 }
 
-func TestLegacyParseRevertUUID(t *testing.T) {
-
-	{
-		uuid, err := legacyParseRevertUUID("revert 4e5dcf80_354b_11eb_82cd_f875a4d24e90")
-		assert.NoError(t, err)
-		assert.Equal(t, "4e5dcf80_354b_11eb_82cd_f875a4d24e90", uuid)
-	}
-	{
-		_, err := legacyParseRevertUUID("revert 4e5dcf80_354b_11eb_82cd_f875a4")
-		assert.Error(t, err)
-	}
-	{
-		_, err := legacyParseRevertUUID("revert vitess_migration '4e5dcf80_354b_11eb_82cd_f875a4d24e90'")
-		assert.Error(t, err)
-	}
-}
-
 func TestParseEnumValues(t *testing.T) {
 	{
 		inputs := []string{