DDL strategy flag '--unsafe-allow-foreign-keys' implies setting 'FORE…

…IGN_KEY_CHECKS=0' in both direct and Online DDL strategies

Signed-off-by: Shlomi Noach <[email protected]>

go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go

@@ -2286,6 +2286,15 @@ func testForeignKeys(t *testing.T) {
 			expectHint:                "child_hint",
 			onlyIfFKOnlineDDLPossible: true,
 		},
+		{
+			name: "add two tables with cyclic fk relationship",
+			sql: `
+				create table t11 (id int primary key, i int, constraint f11 foreign key (i) references t12 (id));
+				create table t12 (id int primary key, i int, constraint f12 foreign key (i) references t11 (id));
+				`,
+			allowForeignKeys: true,
+			expectHint:       "t11",
+		},
 	}
 
 	fkOnlineDDLPossible := false

go/test/endtoend/vtgate/schema/schema_test.go

@@ -108,6 +108,7 @@ func TestSchemaChange(t *testing.T) {
 	testWithDropCreateSchema(t)
 	testDropNonExistentTables(t)
 	testApplySchemaBatch(t)
+	testUnsafeAllowForeignKeys(t)
 	testCreateInvalidView(t)
 	testCopySchemaShards(t, clusterInstance.Keyspaces[0].Shards[0].Vttablets[0].VttabletProcess.TabletPath, 2)
 	testCopySchemaShards(t, fmt.Sprintf("%s/0", keyspaceName), 3)
@@ -252,6 +253,28 @@ func testApplySchemaBatch(t *testing.T) {
 	}
 }
 
+func testUnsafeAllowForeignKeys(t *testing.T) {
+	sqls := `
+		create table t11 (id int primary key, i int, constraint f1101 foreign key (i) references t12 (id) on delete restrict);
+		create table t12 (id int primary key, i int, constraint f1201 foreign key (i) references t11 (id) on delete set null);
+	`
+	{
+		_, err := clusterInstance.VtctldClientProcess.ExecuteCommandWithOutput("ApplySchema", "--ddl-strategy", "direct --allow-zero-in-date", "--sql", sqls, keyspaceName)
+		assert.Error(t, err)
+		checkTables(t, totalTableCount)
+	}
+	{
+		_, err := clusterInstance.VtctldClientProcess.ExecuteCommandWithOutput("ApplySchema", "--ddl-strategy", "direct --unsafe-allow-foreign-keys --allow-zero-in-date", "--sql", sqls, keyspaceName)
+		require.NoError(t, err)
+		checkTables(t, totalTableCount+2)
+	}
+	{
+		_, err := clusterInstance.VtctldClientProcess.ExecuteCommandWithOutput("ApplySchema", "--sql", "drop table t11, t12", keyspaceName)
+		require.NoError(t, err)
+		checkTables(t, totalTableCount)
+	}
+}
+
 // checkTables checks the number of tables in the first two shards.
 func checkTables(t *testing.T, count int) {
 	checkTablesCount(t, clusterInstance.Keyspaces[0].Shards[0].Vttablets[0], count)

go/vt/schemamanager/tablet_executor.go

@@ -490,10 +490,14 @@ func (exec *TabletExecutor) executeOneTablet(
 				return
 			}
 		}
-		result, err = exec.tmc.ExecuteFetchAsDba(ctx, tablet, false, &tabletmanagerdatapb.ExecuteFetchAsDbaRequest{
+		request := &tabletmanagerdatapb.ExecuteFetchAsDbaRequest{
 			Query:   []byte(sql),
 			MaxRows: 10,
-		})
+		}
+		if exec.ddlStrategySetting != nil && exec.ddlStrategySetting.IsAllowForeignKeysFlag() {
+			request.DisableForeignKeyChecks = true
+		}
+		result, err = exec.tmc.ExecuteFetchAsDba(ctx, tablet, false, request)
 
 	}
 	if err != nil {

go/vt/vttablet/onlineddl/executor.go

@@ -646,6 +646,21 @@ func (e *Executor) executeDirectly(ctx context.Context, onlineDDL *schema.Online
 	}
 
 	_ = e.onSchemaMigrationStatus(ctx, onlineDDL.UUID, schema.OnlineDDLStatusRunning, false, progressPctStarted, etaSecondsUnknown, rowsCopiedUnknown, emptyHint)
+	if onlineDDL.StrategySetting().IsAllowForeignKeysFlag() {
+		// Foreign key support is curently "unsafe". We further put the burden on the user
+		// by disabling foreign key checks. With this, the user is able to create cyclic
+		// foreign key references (e.g. t1<->t2) without going through the trouble of
+		// CREATE TABLE t1->CREATE TABLE t2->ALTER TABLE t1 ADD FOREIGN KEY ... REFERENCES ts
+		// Grab current sql_mode value
+		if _, err := conn.ExecuteFetch(`set @vt_onlineddl_foreign_key_checks=@@foreign_key_checks`, 0, false); err != nil {
+			return false, vterrors.Errorf(vtrpcpb.Code_UNKNOWN, "could not read foreign_key_checks: %v", err)
+		}
+		_, err = conn.ExecuteFetch("SET foreign_key_checks=0", 0, false)
+		if err != nil {
+			return false, err
+		}
+		defer conn.ExecuteFetch("SET foreign_key_checks=@vt_onlineddl_foreign_key_checks", 0, false)
+	}
 	_, err = conn.ExecuteFetch(onlineDDL.SQL, 0, false)
 
 	if err != nil {