parser: support index name in FOREIGN KEY clause; Online DDL to reject FK clauses

go/vt/schema/online_ddl.go

@@ -39,16 +39,29 @@ var (
 	onlineDDLGeneratedTableNameRegexp = regexp.MustCompile(`^_[0-f]{8}_[0-f]{4}_[0-f]{4}_[0-f]{4}_[0-f]{12}_([0-9]{14})_(gho|ghc|del|new|vrepl)$`)
 	ptOSCGeneratedTableNameRegexp     = regexp.MustCompile(`^_.*_old$`)
 )
+
 var (
 	// ErrOnlineDDLDisabled is returned when online DDL is disabled, and a user attempts to run an online DDL operation (submit, review, control)
 	ErrOnlineDDLDisabled = errors.New("online DDL is disabled")
+	ErrForeignKeyFound   = errors.New("Foreign key found")
 )
 
 const (
 	SchemaMigrationsTableName = "schema_migrations"
 	RevertActionStr           = "revert"
 )
 
+func errorOnFKWalk(node sqlparser.SQLNode) (kontinue bool, err error) {
+	switch node.(type) {
+	case *sqlparser.CreateTable, *sqlparser.AlterTable,
+		*sqlparser.TableSpec, *sqlparser.AddConstraintDefinition, *sqlparser.ConstraintDefinition:
+		return true, nil
+	case *sqlparser.ForeignKeyDefinition:
+		return false, ErrForeignKeyFound
+	}
+	return false, nil
+}
+
 // MigrationBasePath is the root for all schema migration entries
 func MigrationBasePath() string {
 	return migrationBasePath
@@ -154,7 +167,6 @@ func NewOnlineDDLs(keyspace string, ddlStmt sqlparser.DDLStatement, ddlStrategyS
 		if err := appendOnlineDDL(ddlStmt.GetTable().Name.String(), ddlStmt); err != nil {
 			return nil, err
 		}
-		return onlineDDLs, nil
 	case *sqlparser.DropTable:
 		tables := ddlStmt.GetFromTables()
 		for _, table := range tables {
@@ -163,10 +175,28 @@ func NewOnlineDDLs(keyspace string, ddlStmt sqlparser.DDLStatement, ddlStrategyS
 				return nil, err
 			}
 		}
-		return onlineDDLs, nil
 	default:
 		return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported statement for Online DDL: %v", sqlparser.String(ddlStmt))
 	}
+
+	{
+		// SQL statement sanity checks:
+		switch ddlStmt := ddlStmt.(type) {
+		case *sqlparser.AlterTable:
+			if len(ddlStmt.AlterOptions) == 0 {
+				return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "NewOnlineDDL: cannot parse statement: %v", sqlparser.String(ddlStmt))
+			}
+		}
+
+		if !ddlStmt.IsFullyParsed() {
+			return nil, vterrors.NewErrorf(vtrpcpb.Code_INVALID_ARGUMENT, vterrors.SyntaxError, "NewOnlineDDL: cannot parse statement: %v", sqlparser.String(ddlStmt))
+		}
+
+		if err := sqlparser.Walk(errorOnFKWalk, ddlStmt); err == ErrForeignKeyFound {
+			return nil, vterrors.Errorf(vtrpcpb.Code_ABORTED, "foreign key constraint are not supported in online DDL")
+		}
+	}
+	return onlineDDLs, nil
 }
 
 // NewOnlineDDL creates a schema change request with self generated UUID and RequestTime
@@ -216,7 +246,7 @@ func NewOnlineDDL(keyspace string, table string, sql string, ddlStrategySetting
 			switch stmt := stmt.(type) {
 			case sqlparser.DDLStatement:
 				if !stmt.IsFullyParsed() {
-					return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "NewOnlineDDL: cannot fully parse statement %v", sqlparser.String(stmt))
+					return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "NewOnlineDDL: cannot parse statement: %v", sqlparser.String(stmt))
 				}
 				stmt.SetComments(comments)
 			case *sqlparser.RevertMigration:

go/vt/schema/online_ddl_test.go

@@ -265,6 +265,7 @@ func TestNewOnlineDDLs(t *testing.T) {
 		"truncate table t":                              {isError: true},
 		"drop view t":                                   {isError: true},
 		"rename table t to t1":                          {isError: true},
+		"alter table corder add FOREIGN KEY my_fk(customer_id) reference customer(customer_id)": {isError: true},
 	}
 	migrationContext := "354b-11eb-82cd-f875a4d24e90"
 	for query, expect := range tests {

go/vt/sqlparser/ast.go

@@ -1530,6 +1530,7 @@ type (
 	// ForeignKeyDefinition describes a foreign key in a CREATE TABLE statement
 	ForeignKeyDefinition struct {
 		Source            Columns
+		IndexName         ColIdent
 		ReferencedTable   TableName
 		ReferencedColumns Columns
 		OnDelete          ReferenceAction

go/vt/sqlparser/ast_format.go

@@ -593,7 +593,7 @@ func (a ReferenceAction) Format(buf *TrackedBuffer) {
 
 // Format formats the node.
 func (f *ForeignKeyDefinition) Format(buf *TrackedBuffer) {
-	buf.astPrintf(f, "foreign key %v references %v %v", f.Source, f.ReferencedTable, f.ReferencedColumns)
+	buf.astPrintf(f, "foreign key %v%v references %v %v", f.IndexName, f.Source, f.ReferencedTable, f.ReferencedColumns)
 	if f.OnDelete != DefaultAction {
 		buf.astPrintf(f, " on delete %v", f.OnDelete)
 	}

go/vt/sqlparser/parse_test.go

@@ -1034,6 +1034,8 @@ var (
 		input: "alter table a add fulltext index idx (id)",
 	}, {
 		input: "alter table a add foreign key (id) references f (id)",
+	}, {
+		input: "alter table a add foreign key the_idx(id) references f (id)",
 	}, {
 		input: "alter table a add primary key (id)",
 	}, {