Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VReplication: Properly Handle FK Constraints When Deferring Secondary Keys #14543

Merged
merged 3 commits into from
Dec 4, 2023
Merged

VReplication: Properly Handle FK Constraints When Deferring Secondary Keys #14543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bef521
Add unit test case
mattlord Nov 17, 2023
3fcdb55
Correct unit test and add fix for the bug
mattlord Nov 17, 2023
18a9896
Add more unit test cases
mattlord Nov 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion go/vt/vttablet/tabletmanager/vreplication/vreplicator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,27 @@ func (vr *vreplicator) getTableSecondaryKeys(ctx context.Context, tableName stri
return nil, fmt.Errorf("could not determine CREATE TABLE statement from table schema %q", tableSchema)
}

for _, index := range createTable.GetTableSpec().Indexes {
tableSpec := createTable.GetTableSpec()
fkIndexCols := make(map[string]bool)
for _, constraint := range tableSpec.Constraints {
if fkDef, ok := constraint.Details.(*sqlparser.ForeignKeyDefinition); ok {
fkCols := make([]string, len(fkDef.Source))
for i, fkCol := range fkDef.Source {
fkCols[i] = fkCol.Lowered()
}
fkIndexCols[strings.Join(fkCols, ",")] = true
}
}
for _, index := range tableSpec.Indexes {
if index.Info.Type != sqlparser.IndexTypePrimary {
cols := make([]string, len(index.Columns))
for i, col := range index.Columns {
cols[i] = col.Column.Lowered()
}
if fkIndexCols[strings.Join(cols, ",")] {
// This index is needed for a FK constraint so we cannot drop it.
continue
}
secondaryKeys = append(secondaryKeys, index)
}
}
Expand Down
59 changes: 57 additions & 2 deletions go/vt/vttablet/tabletmanager/vreplication/vreplicator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ import (
"vitess.io/vitess/go/vt/binlog/binlogplayer"
"vitess.io/vitess/go/vt/dbconfigs"
"vitess.io/vitess/go/vt/mysqlctl"
"vitess.io/vitess/go/vt/schemadiff"

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
"vitess.io/vitess/go/vt/schemadiff"
)

func TestRecalculatePKColsInfoByColumnNames(t *testing.T) {
Expand Down Expand Up @@ -256,6 +257,7 @@ func TestDeferSecondaryKeys(t *testing.T) {
wantStashErr string
wantExecErr string
expectFinalSchemaDiff bool
preStashHook func() error
postStashHook func() error
}{
{
Expand Down Expand Up @@ -297,6 +299,54 @@ func TestDeferSecondaryKeys(t *testing.T) {
actionDDL: "alter table %s.t1 add key c1 (c1), add key c2 (c2)",
WorkflowType: int32(binlogdatapb.VReplicationWorkflowType_MoveTables),
},
{
name: "2SK:1FK",
tableName: "t1",
initialDDL: "create table t1 (id int not null, c1 int default null, c2 int default null, t2_id int not null, primary key (id), key c1 (c1), key c2 (c2), foreign key (t2_id) references t2 (id))",
// Secondary key t2_id is needed to enforce the FK constraint so we do not drop it.
strippedDDL: "create table t1 (id int not null, c1 int default null, c2 int default null, t2_id int not null, primary key (id), key t2_id (t2_id), constraint t1_ibfk_1 foreign key (t2_id) references t2 (id))",
actionDDL: "alter table %s.t1 add key c1 (c1), add key c2 (c2)",
WorkflowType: int32(binlogdatapb.VReplicationWorkflowType_MoveTables),
preStashHook: func() error {
if _, err := dbClient.ExecuteFetch("drop table if exists t2", 1); err != nil {
return err
}
_, err = dbClient.ExecuteFetch("create table t2 (id int not null, c1 int not null, primary key (id))", 1)
return err
},
},
{
name: "3SK:2FK",
tableName: "t1",
initialDDL: "create table t1 (id int not null, id2 int default null, c1 int default null, c2 int default null, c3 int default null, t2_id int not null, t2_id2 int not null, primary key (id), key c1 (c1), key c2 (c2), foreign key (t2_id) references t2 (id), key c3 (c3), foreign key (t2_id2) references t2 (id2))",
// Secondary keys t2_id and t2_id2 are needed to enforce the FK constraint so we do not drop them.
strippedDDL: "create table t1 (id int not null, id2 int default null, c1 int default null, c2 int default null, c3 int default null, t2_id int not null, t2_id2 int not null, primary key (id), key t2_id (t2_id), key t2_id2 (t2_id2), constraint t1_ibfk_1 foreign key (t2_id) references t2 (id), constraint t1_ibfk_2 foreign key (t2_id2) references t2 (id2))",
actionDDL: "alter table %s.t1 add key c1 (c1), add key c2 (c2), add key c3 (c3)",
WorkflowType: int32(binlogdatapb.VReplicationWorkflowType_MoveTables),
preStashHook: func() error {
if _, err := dbClient.ExecuteFetch("drop table if exists t2", 1); err != nil {
return err
}
_, err = dbClient.ExecuteFetch("create table t2 (id int not null, id2 int default null, c1 int not null, primary key (id), key (id2))", 1)
return err
},
},
{
name: "5SK:2FK_multi-column",
tableName: "t1",
initialDDL: "create table t1 (id int not null, id2 int default null, c1 int default null, c2 int default null, c3 int default null, t2_id int not null, t2_id2 int not null, primary key (id), key c1 (c1), key c2 (c2), key t2_cs (c1,c2), key t2_ids (t2_id,t2_id2), foreign key (t2_id,t2_id2) references t2 (id, id2), key c3 (c3), foreign key (c1, c2) references t2 (c1, c2))",
// Secondary keys t2_ids and t2_cs are needed to enforce the FK constraint so we do not drop them.
strippedDDL: "create table t1 (id int not null, id2 int default null, c1 int default null, c2 int default null, c3 int default null, t2_id int not null, t2_id2 int not null, primary key (id), key t2_cs (c1,c2), key t2_ids (t2_id,t2_id2), constraint t1_ibfk_1 foreign key (t2_id, t2_id2) references t2 (id, id2), constraint t1_ibfk_2 foreign key (c1, c2) references t2 (c1, c2))",
actionDDL: "alter table %s.t1 add key c1 (c1), add key c2 (c2), add key c3 (c3)",
WorkflowType: int32(binlogdatapb.VReplicationWorkflowType_MoveTables),
preStashHook: func() error {
if _, err := dbClient.ExecuteFetch("drop table if exists t2", 1); err != nil {
return err
}
_, err = dbClient.ExecuteFetch("create table t2 (id int not null, id2 int not null, c1 int not null, c2 int not null, primary key (id,id2), key (c1,c2))", 1)
return err
},
},
{
name: "2tSK",
tableName: "t1",
Expand Down Expand Up @@ -425,6 +475,11 @@ func TestDeferSecondaryKeys(t *testing.T) {
// MoveTables and Reshard workflows.
vr.WorkflowType = tcase.WorkflowType

if tcase.preStashHook != nil {
err = tcase.preStashHook()
require.NoError(t, err, "error executing pre stash hook: %v", err)
}

// Create the table.
_, err := dbClient.ExecuteFetch(tcase.initialDDL, 1)
require.NoError(t, err)
Expand Down Expand Up @@ -456,7 +511,7 @@ func TestDeferSecondaryKeys(t *testing.T) {

if tcase.postStashHook != nil {
err = tcase.postStashHook()
require.NoError(t, err)
require.NoError(t, err, "error executing post stash hook: %v", err)

// We should still NOT have any secondary keys because there's still
// a running controller/vreplicator in the copy phase.
Expand Down
Loading