From 6caaa92f709b4885b3d82e0f83144dde00d6aba6 Mon Sep 17 00:00:00 2001
From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Date: Tue, 31 Oct 2023 07:59:21 +0200
Subject: [PATCH] schemadiff: identify a FK sequential execution scenario where
 adding an index over foreign key parent referenced columns

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
---
 go/vt/schemadiff/schema.go           | 17 ++++++++++++++++-
 go/vt/schemadiff/schema_diff_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go
index a9ef60fbb27..145ddd0c46f 100644
--- a/go/vt/schemadiff/schema.go
+++ b/go/vt/schemadiff/schema.go
@@ -832,7 +832,9 @@ func (s *Schema) SchemaDiff(other *Schema, hints *DiffHints) (*SchemaDiff, error
 							// The current diff is ALTER TABLE ... ADD FOREIGN KEY
 							// and the parent table also has an ALTER TABLE.
 							// so if the parent's ALTER in any way modifies the referenced FK columns, that's
-							// a sequential execution dependency
+							// a sequential execution dependency.
+							// Also, if there is no index on the parent's referenced columns, and a migration adds an index
+							// on those columns, that's a sequential execution dependency.
 							referencedColumnNames := map[string]bool{}
 							for _, referencedColumn := range fk.ReferenceDefinition.ReferencedColumns {
 								referencedColumnNames[referencedColumn.Lowered()] = true
@@ -854,6 +856,19 @@ func (s *Schema) SchemaDiff(other *Schema, hints *DiffHints) (*SchemaDiff, error
 									if referencedColumnNames[node.Name.Name.Lowered()] {
 										schemaDiff.addDep(diff, parentDiff, DiffDependencySequentialExecution)
 									}
+								case *sqlparser.AddIndexDefinition:
+									referencedTableEntity, _ := parentDiff.Entities()
+									// We _know_ the type is *CreateTableEntity
+									referencedTable, _ := referencedTableEntity.(*CreateTableEntity)
+									if indexCoversColumnsInOrder(node.IndexDefinition, fk.ReferenceDefinition.ReferencedColumns) {
+										// This diff adds an index covering referenced columns
+										if !referencedTable.columnsCoveredByInOrderIndex(fk.ReferenceDefinition.ReferencedColumns) {
+											// And there was no earlier index on referenced columns. So this is a new index.
+											// In MySQL, you can't add a foreign key constraint on a child, before the parent
+											// has an index of referenced columns. This is a sequential dependency.
+											schemaDiff.addDep(diff, parentDiff, DiffDependencySequentialExecution)
+										}
+									}
 								}
 								return true, nil
 							}, parentDiff.Statement())
diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go
index d835177c795..ee5a5663a76 100644
--- a/go/vt/schemadiff/schema_diff_test.go
+++ b/go/vt/schemadiff/schema_diff_test.go
@@ -672,6 +672,34 @@ func TestSchemaDiff(t *testing.T) {
 			expectDeps:  1,
 			sequential:  true,
 			entityOrder: []string{"t2", "t1"},
+		}, {
+			name: "add index on parent. add FK to index column",
+			toQueries: []string{
+				"create table t1 (id int primary key, info int not null, key info_idx(info));",
+				"create table t2 (id int primary key, ts timestamp, t1_info int not null, constraint parent_info_fk foreign key (t1_info) references t1 (info));",
+				"create view v1 as select id from t1",
+			},
+			expectDiffs: 2,
+			expectDeps:  1,
+			sequential:  true,
+			entityOrder: []string{"t1", "t2"},
+		},
+		{
+			name: "add index on parent with existing index. add FK to index column",
+			fromQueries: []string{
+				"create table t1 (id int primary key, info int not null, key existing_info_idx(info));",
+				"create table t2 (id int primary key, ts timestamp);",
+				"create view v1 as select id from t1",
+			},
+			toQueries: []string{
+				"create table t1 (id int primary key, info int not null, key existing_info_idx(info), key info_idx(info));",
+				"create table t2 (id int primary key, ts timestamp, t1_info int not null, constraint parent_info_fk foreign key (t1_info) references t1 (info));",
+				"create view v1 as select id from t1",
+			},
+			expectDiffs: 2,
+			expectDeps:  1,
+			sequential:  false,
+			entityOrder: []string{"t1", "t2"},
 		},
 		{
 			name: "drop fk",