Skip to content

Commit

Permalink
schemadiff: identify a FK sequential execution scenario where adding …
Browse files Browse the repository at this point in the history
…an index over foreign key parent referenced columns

Signed-off-by: Shlomi Noach <[email protected]>
  • Loading branch information
shlomi-noach committed Oct 31, 2023
1 parent 9c47e63 commit 6caaa92
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
17 changes: 16 additions & 1 deletion go/vt/schemadiff/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
Expand Down
28 changes: 28 additions & 0 deletions go/vt/schemadiff/schema_diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 6caaa92

Please sign in to comment.