Merge pull request #8228 from planetscale/online-ddl-vrpel-suite-fail-fk

Online DDL/VReplication: fail on existence of FOREIGN KEY

go/test/endtoend/onlineddl/vrepl_suite/onlineddl_vrepl_suite_test.go

@@ -190,7 +190,7 @@ func testSingle(t *testing.T, testName string) {
 	_ = mysqlExec(t, sqlModeQuery, "")
 	_ = mysqlExec(t, "set @@global.event_scheduler=1", "")
 
-	_ = mysqlExec(t, fmt.Sprintf("drop table if exists %s, %s, %s", tableName, beforeTableName, afterTableName), "")
+	_ = mysqlExec(t, fmt.Sprintf("drop table if exists %s_child, %s, %s_parent, %s, %s;", tableName, tableName, tableName, beforeTableName, afterTableName), "")
 	_ = mysqlExec(t, fmt.Sprintf("drop event if exists %s", eventName), "")
 
 	{

go/test/endtoend/onlineddl/vrepl_suite/testdata/discard-fk/create.sql → go/test/endtoend/onlineddl/vrepl_suite/testdata/fail-fk-child/create.sql

@@ -1,7 +1,9 @@
-drop table if exists gh_ost_test_child;
+set session foreign_key_checks=0;
+drop table if exists onlineddl_test_child;
 drop table if exists onlineddl_test;
-drop table if exists gh_ost_test_fk_parent;
-create table gh_ost_test_fk_parent (
+drop table if exists onlineddl_test_parent;
+set session foreign_key_checks=1;
+create table onlineddl_test_parent (
   id int auto_increment,
   ts timestamp,
   primary key(id)
@@ -11,10 +13,10 @@ create table onlineddl_test (
   i int not null,
   parent_id int not null,
   primary key(id),
-  constraint test_fk foreign key (parent_id) references gh_ost_test_fk_parent (id) on delete no action
+  constraint test_fk foreign key (parent_id) references onlineddl_test_parent (id) on delete no action
 ) auto_increment=1;
 
-insert into gh_ost_test_fk_parent (id) values (1),(2),(3);
+insert into onlineddl_test_parent (id) values (1),(2),(3);
 
 drop event if exists onlineddl_test;
 delimiter ;;

go/test/endtoend/onlineddl/vrepl_suite/testdata/fail-fk-child/expect_failure

@@ -0,0 +1 @@
+foreign key constraints are not supported in online DDL

go/test/endtoend/onlineddl/vrepl_suite/testdata/fail-fk-parent/create.sql

@@ -0,0 +1,34 @@
+set session foreign_key_checks=0;
+drop table if exists onlineddl_test_child;
+drop table if exists onlineddl_test;
+drop table if exists onlineddl_test_parent;
+set session foreign_key_checks=1;
+create table onlineddl_test (
+  id int auto_increment,
+  ts timestamp,
+  primary key(id)
+);
+create table onlineddl_test_child (
+  id int auto_increment,
+  i int not null,
+  parent_id int not null,
+  primary key(id),
+  constraint test_fk foreign key (parent_id) references onlineddl_test (id) on delete no action
+) auto_increment=1;
+
+insert into onlineddl_test (id) values (1),(2),(3);
+
+drop event if exists onlineddl_test;
+delimiter ;;
+create event onlineddl_test
+  on schedule every 1 second
+  starts current_timestamp
+  ends current_timestamp + interval 60 second
+  on completion not preserve
+  enable
+  do
+begin
+  insert into onlineddl_test_child values (null, 11, 1);
+  insert into onlineddl_test_child values (null, 13, 2);
+  insert into onlineddl_test_child values (null, 17, 3);
+end ;;

go/test/endtoend/onlineddl/vrepl_suite/testdata/fail-fk-parent/expect_failure

@@ -0,0 +1 @@
+foreign key constraints are not supported in online DDL

go/vt/vttablet/onlineddl/executor.go

@@ -476,6 +476,33 @@ func (e *Executor) executeDirectly(ctx context.Context, onlineDDL *schema.Online
 	return acceptableErrorCodeFound, nil
 }
 
+// validateTableForAlterAction checks whether a table is good to undergo a ALTER operation. It returns detailed error if not.
+func (e *Executor) validateTableForAlterAction(ctx context.Context, onlineDDL *schema.OnlineDDL) (err error) {
+	// Validate table does not participate in foreign key relationship:
+	for _, fkQuery := range []string{selSelectCountFKParentConstraints, selSelectCountFKChildConstraints} {
+		query, err := sqlparser.ParseAndBind(fkQuery,
+			sqltypes.StringBindVariable(onlineDDL.Schema),
+			sqltypes.StringBindVariable(onlineDDL.Table),
+		)
+		if err != nil {
+			return err
+		}
+		r, err := e.execQuery(ctx, query)
+		if err != nil {
+			return err
+		}
+		row := r.Named().Row()
+		if row == nil {
+			return vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "unexpected result from INFORMATION_SCHEMA.KEY_COLUMN_USAGE query: %s", query)
+		}
+		countFKConstraints := row.AsInt64("num_fk_constraints", 0)
+		if countFKConstraints > 0 {
+			return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "table %s participates in FOREIGN KEY constraint. foreign key constraints are not supported in online DDL", onlineDDL.Table)
+		}
+	}
+	return nil
+}
+
 // terminateVReplMigration stops vreplication, then removes the _vt.vreplication entry for the given migration
 func (e *Executor) terminateVReplMigration(ctx context.Context, uuid string) error {
 	tmClient := tmclient.NewTabletManagerClient()
@@ -739,6 +766,11 @@ func (e *Executor) ExecuteWithVReplication(ctx context.Context, onlineDDL *schem
 	if err := v.analyze(ctx, conn); err != nil {
 		return err
 	}
+	if revertMigration == nil {
+		if err := e.validateTableForAlterAction(ctx, onlineDDL); err != nil {
+			return err
+		}
+	}
 	if err := e.updateArtifacts(ctx, onlineDDL.UUID, v.targetTable); err != nil {
 		return err
 	}

go/vt/vttablet/onlineddl/schema.go

@@ -293,6 +293,23 @@ const (
 			AND ACTION_TIMING='AFTER'
 			AND LEFT(TRIGGER_NAME, 7)='pt_osc_'
 		`
+	selSelectCountFKParentConstraints = `
+		SELECT
+			COUNT(*) as num_fk_constraints
+		FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE
+		WHERE
+			REFERENCED_TABLE_SCHEMA=%a AND REFERENCED_TABLE_NAME=%a
+			AND REFERENCED_TABLE_NAME IS NOT NULL
+	`
+	selSelectCountFKChildConstraints = `
+		SELECT
+			COUNT(*) as num_fk_constraints
+		FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE
+		WHERE
+			TABLE_SCHEMA=%a AND TABLE_NAME=%a
+			AND REFERENCED_TABLE_NAME IS NOT NULL
+	`
+
 	sqlDropTrigger       = "DROP TRIGGER IF EXISTS `%a`.`%a`"
 	sqlShowTablesLike    = "SHOW TABLES LIKE '%a'"
 	sqlCreateTableLike   = "CREATE TABLE `%a` LIKE `%a`"