From 1c467275d07167c13cb69782ce64c6d116a44d9d Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:47:02 +0300 Subject: [PATCH 1/7] schemadiff: adding a FK dependency unit test Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_diff_test.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go index 3dc1ab291fd..75b5599f39c 100644 --- a/go/vt/schemadiff/schema_diff_test.go +++ b/go/vt/schemadiff/schema_diff_test.go @@ -977,6 +977,22 @@ func TestSchemaDiff(t *testing.T) { conflictingDiffs: 2, instantCapability: InstantDDLCapabilityImpossible, }, + { + name: "add and drop FK, add and drop respective tables", + fromQueries: []string{ + "create table t1 (id int primary key, p int, key p_idx (p));", + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t1 (p) on delete no action);", + }, + toQueries: []string{ + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t3 (p) on delete no action);", + "create table t3 (id int primary key, p int, key p_idx (p));", + }, + expectDiffs: 3, + expectDeps: 2, + sequential: true, + entityOrder: []string{"t3", "t2", "t1"}, + instantCapability: InstantDDLCapabilityImpossible, + }, { name: "two identical foreign keys in table, drop one", fromQueries: []string{ @@ -1051,7 +1067,7 @@ func TestSchemaDiff(t *testing.T) { for _, dep := range deps { depsKeys = append(depsKeys, dep.hashKey()) } - assert.Equalf(t, tc.expectDeps, len(deps), "found deps: %v", depsKeys) + assert.Equalf(t, tc.expectDeps, len(deps), "found %v deps: %v", len(depsKeys), depsKeys) assert.Equal(t, tc.sequential, schemaDiff.HasSequentialExecutionDependencies()) orderedDiffs, err := schemaDiff.OrderedDiffs(ctx) From b2d882a9b576a5fa49aaeffa64af5ecc5a2c6474 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:59:11 +0300 Subject: [PATCH 2/7] code comment Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_diff_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go index 75b5599f39c..cf8a85fcff2 100644 --- a/go/vt/schemadiff/schema_diff_test.go +++ b/go/vt/schemadiff/schema_diff_test.go @@ -988,7 +988,7 @@ func TestSchemaDiff(t *testing.T) { "create table t3 (id int primary key, p int, key p_idx (p));", }, expectDiffs: 3, - expectDeps: 2, + expectDeps: 2, // [alter t2]/[drop t1], [alter t2]/[create t3] sequential: true, entityOrder: []string{"t3", "t2", "t1"}, instantCapability: InstantDDLCapabilityImpossible, From 2b5da08dcf6b2944f3fcfe4af89a3cabdbce8867 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 6 Jun 2024 14:58:21 +0300 Subject: [PATCH 3/7] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_diff_test.go | 97 ++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 5 deletions(-) diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go index cf8a85fcff2..28e81dd4a84 100644 --- a/go/vt/schemadiff/schema_diff_test.go +++ b/go/vt/schemadiff/schema_diff_test.go @@ -195,7 +195,7 @@ func TestPermutations(t *testing.T) { allDiffs := schemaDiff.UnorderedDiffs() originalSingleString := toSingleString(allDiffs) numEquals := 0 - earlyBreak, err := permutateDiffs(ctx, allDiffs, func(pdiffs []EntityDiff) (earlyBreak bool) { + earlyBreak, err := permutateDiffs(ctx, allDiffs, hints, func(pdiffs []EntityDiff, hints *DiffHints) (earlyBreak bool) { defer func() { iteration++ }() // cover all permutations singleString := toSingleString(pdiffs) @@ -218,7 +218,7 @@ func TestPermutations(t *testing.T) { allPerms := map[string]bool{} allDiffs := schemaDiff.UnorderedDiffs() originalSingleString := toSingleString(allDiffs) - earlyBreak, err := permutateDiffs(ctx, allDiffs, func(pdiffs []EntityDiff) (earlyBreak bool) { + earlyBreak, err := permutateDiffs(ctx, allDiffs, hints, func(pdiffs []EntityDiff, hints *DiffHints) (earlyBreak bool) { // Single visit allPerms[toSingleString(pdiffs)] = true // First permutation should be the same as original @@ -244,8 +244,9 @@ func TestPermutationsContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() + hints := &DiffHints{RangeRotationStrategy: RangeRotationDistinctStatements} allDiffs := []EntityDiff{&DropViewEntityDiff{}} - earlyBreak, err := permutateDiffs(ctx, allDiffs, func(pdiffs []EntityDiff) (earlyBreak bool) { + earlyBreak, err := permutateDiffs(ctx, allDiffs, hints, func(pdiffs []EntityDiff, hints *DiffHints) (earlyBreak bool) { return false }) assert.True(t, earlyBreak) // proves that termination was due to context cancel @@ -676,6 +677,74 @@ func TestSchemaDiff(t *testing.T) { instantCapability: InstantDDLCapabilityIrrelevant, fkStrategy: ForeignKeyCheckStrategyIgnore, }, + { + name: "two table cycle with create, strict", + fromQueries: append(createQueries, + "create table t11 (id int primary key, i0 int);", + ), + toQueries: append( + createQueries, + "create table t12 (id int primary key, i int, constraint f1201 foreign key (i) references t11 (i) on delete set null);", + "create table t11 (id int primary key, i0 int, i int, key (i), constraint f1101 foreign key (i) references t12 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + sequential: true, + instantCapability: InstantDDLCapabilityIrrelevant, + fkStrategy: ForeignKeyCheckStrategyStrict, + expectOrderedError: "no valid applicable order for diffs", + }, + { + name: "two table cycle with create, strict, changed lexicographic order", + fromQueries: append(createQueries, + "create table t12 (id int primary key, i0 int);", + ), + toQueries: append( + createQueries, + "create table t11 (id int primary key, i int, constraint f1201 foreign key (i) references t12 (i) on delete set null);", + "create table t12 (id int primary key, i0 int, i int, key (i), constraint f1101 foreign key (i) references t11 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyStrict, + expectOrderedError: "no valid applicable order for diffs", + }, + { + name: "two table cycle with create, ignore", + fromQueries: append(createQueries, + "create table t11 (id int primary key, i0 int);", + ), + toQueries: append( + createQueries, + "create table t12 (id int primary key, i int, constraint f1201 foreign key (i) references t11 (i) on delete set null);", + "create table t11 (id int primary key, i0 int, i int, key (i), constraint f1101 foreign key (i) references t12 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + entityOrder: []string{"t11", "t12"}, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyIgnore, + }, + { + name: "two table cycle with create, ignore, changed lexicographic order", + fromQueries: append(createQueries, + "create table t12 (id int primary key, i0 int);", + ), + toQueries: append( + createQueries, + "create table t11 (id int primary key, i int, constraint f1201 foreign key (i) references t12 (i) on delete set null);", + "create table t12 (id int primary key, i0 int, i int, key (i), constraint f1101 foreign key (i) references t11 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + entityOrder: []string{"t12", "t11"}, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyIgnore, + }, { name: "add FK", toQueries: []string{ @@ -978,7 +1047,7 @@ func TestSchemaDiff(t *testing.T) { instantCapability: InstantDDLCapabilityImpossible, }, { - name: "add and drop FK, add and drop respective tables", + name: "add and drop FK, add and drop respective tables, fk strict", fromQueries: []string{ "create table t1 (id int primary key, p int, key p_idx (p));", "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t1 (p) on delete no action);", @@ -992,6 +1061,24 @@ func TestSchemaDiff(t *testing.T) { sequential: true, entityOrder: []string{"t3", "t2", "t1"}, instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyStrict, + }, + { + name: "add and drop FK, add and drop respective tables, fk ignore", + fromQueries: []string{ + "create table t1 (id int primary key, p int, key p_idx (p));", + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t1 (p) on delete no action);", + }, + toQueries: []string{ + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t3 (p) on delete no action);", + "create table t3 (id int primary key, p int, key p_idx (p));", + }, + expectDiffs: 3, + expectDeps: 2, // [alter t2]/[drop t1], [alter t2]/[create t3] + sequential: true, + entityOrder: []string{"t3", "t2", "t1"}, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyIgnore, }, { name: "two identical foreign keys in table, drop one", @@ -1109,7 +1196,7 @@ func TestSchemaDiff(t *testing.T) { require.NoError(t, err) } instantCapability := schemaDiff.InstantDDLCapability() - assert.Equal(t, tc.instantCapability, instantCapability) + assert.Equal(t, tc.instantCapability, instantCapability, "for instant capability") }) } From 1e837c81bcffa61a6c76723caf3866ce8c7276bc Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 6 Jun 2024 14:58:47 +0300 Subject: [PATCH 4/7] comment Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go index efad76d9a33..2e25f9cf3d6 100644 --- a/go/vt/schemadiff/schema.go +++ b/go/vt/schemadiff/schema.go @@ -234,7 +234,7 @@ func (s *Schema) normalize(hints *DiffHints) error { // already handled; skip continue } - // Not handled. Is this view dependent on already handled objects? + // Not handled. Does this table reference an already handled table? referencedTableNames := getForeignKeyParentTableNames(t.CreateTable) if len(referencedTableNames) > 0 { s.foreignKeyChildren = append(s.foreignKeyChildren, t) From 35845052bf648ce89149dfd5746f7e1fe65e41df Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 6 Jun 2024 15:00:07 +0300 Subject: [PATCH 5/7] refactor permutations logic Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_diff.go | 62 +++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/go/vt/schemadiff/schema_diff.go b/go/vt/schemadiff/schema_diff.go index 3fbc1e6c9d3..4fc6e7cec8a 100644 --- a/go/vt/schemadiff/schema_diff.go +++ b/go/vt/schemadiff/schema_diff.go @@ -88,27 +88,27 @@ Modified to have an early break // permutateDiffs calls `callback` with each permutation of a. If the function returns `true`, that means // the callback has returned `true` for an early break, thus possibly not all permutations have been evaluated. -func permutateDiffs(ctx context.Context, diffs []EntityDiff, callback func([]EntityDiff) (earlyBreak bool)) (earlyBreak bool, err error) { +func permutateDiffs(ctx context.Context, diffs []EntityDiff, hints *DiffHints, callback func([]EntityDiff, *DiffHints) (earlyBreak bool)) (earlyBreak bool, err error) { if len(diffs) == 0 { return false, nil } // Sort by a heuristic (DROPs first, ALTERs next, CREATEs last). This ordering is then used first in the permutation // search and serves as seed for the rest of permutations. - return permDiff(ctx, diffs, callback, 0) + return permDiff(ctx, diffs, hints, callback, 0) } // permDiff is a recursive function to permutate given `a` and call `callback` for each permutation. // If `callback` returns `true`, then so does this function, and this indicates a request for an early // break, in which case this function will not be called again. -func permDiff(ctx context.Context, a []EntityDiff, callback func([]EntityDiff) (earlyBreak bool), i int) (earlyBreak bool, err error) { +func permDiff(ctx context.Context, a []EntityDiff, hints *DiffHints, callback func([]EntityDiff, *DiffHints) (earlyBreak bool), i int) (earlyBreak bool, err error) { if err := ctx.Err(); err != nil { return true, err // early break } if i > len(a) { - return callback(a), nil + return callback(a, hints), nil } - if brk, err := permDiff(ctx, a, callback, i+1); brk { + if brk, err := permDiff(ctx, a, hints, callback, i+1); brk { return true, err } for j := i + 1; j < len(a); j++ { @@ -150,7 +150,7 @@ func permDiff(ctx context.Context, a []EntityDiff, callback func([]EntityDiff) ( } // End of optimization a[i], a[j] = a[j], a[i] - if brk, err := permDiff(ctx, a, callback, i+1); brk { + if brk, err := permDiff(ctx, a, hints, callback, i+1); brk { return true, err } a[i], a[j] = a[j], a[i] @@ -315,24 +315,48 @@ func (d *SchemaDiff) OrderedDiffs(ctx context.Context) ([]EntityDiff, error) { // We will now permutate the diffs in this equivalence class, and hopefully find // a valid permutation (one where if we apply the diffs in-order, the schema remains valid throughout the process) - foundValidPathForClass, err := permutateDiffs(ctx, classDiffs, func(permutatedDiffs []EntityDiff) bool { - permutationSchema := lastGoodSchema.copy() - // We want to apply the changes one by one, and validate the schema after each change - for i := range permutatedDiffs { - // apply inline - if err := permutationSchema.apply(permutatedDiffs[i:i+1], d.hints); err != nil { - // permutation is invalid - return false // continue searching + tryPermutateDiffs := func(hints *DiffHints) (bool, error) { + return permutateDiffs(ctx, classDiffs, hints, func(permutatedDiffs []EntityDiff, hints *DiffHints) bool { + permutationSchema := lastGoodSchema.copy() + // We want to apply the changes one by one, and validate the schema after each change + for i := range permutatedDiffs { + // apply inline + if err := permutationSchema.apply(permutatedDiffs[i:i+1], hints); err != nil { + // permutation is invalid + return false // continue searching + } + } + // Good news, we managed to apply all of the permutations! + orderedDiffs = append(orderedDiffs, permutatedDiffs...) + lastGoodSchema = permutationSchema + return true // early break! No need to keep searching + }) + } + tryPermutateDiffsAcrossHints := func() (found bool, err error) { + for _, fkStrategy := range []int{ForeignKeyCheckStrategyStrict, ForeignKeyCheckStrategyIgnore} { + hints := *d.hints + hints.ForeignKeyCheckStrategy = fkStrategy + found, err = tryPermutateDiffs(&hints) + if fkStrategy == d.hints.ForeignKeyCheckStrategy { + // end of the line. + return found, err + } + if err != nil { + continue + } + if found { + return true, nil } } - // Good news, we managed to apply all of the permutations! - orderedDiffs = append(orderedDiffs, permutatedDiffs...) - lastGoodSchema = permutationSchema - return true // early break! No need to keep searching - }) + return found, err + } + foundValidPathForClass, err := tryPermutateDiffsAcrossHints() if err != nil { return nil, err } + // We prefer stricter strategy, because that gives best chance of finding a valid path. + // So on best effort basis, we try to find a valid path starting with the strictest strategy, easing + // to more relaxed ones, but never below the preconfigured if !foundValidPathForClass { // In this equivalence class, there is no valid permutation. We cannot linearize the diffs. return nil, &ImpossibleApplyDiffOrderError{ From 78234e0a29db9a124a2973140e4f414709238295 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 6 Jun 2024 17:55:56 +0300 Subject: [PATCH 6/7] Implement ForeignKeyCheckStrategyCreateTableFirst; use a strictest-to-desired search path, that first attempts to echieve a dependency resolution with strictest means, but if impossible, resotrs to more relaxed constraints Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_diff.go | 34 ++++++++--- go/vt/schemadiff/schema_diff_test.go | 85 ++++++++++++++++++++++++++++ go/vt/schemadiff/types.go | 1 + 3 files changed, 113 insertions(+), 7 deletions(-) diff --git a/go/vt/schemadiff/schema_diff.go b/go/vt/schemadiff/schema_diff.go index 4fc6e7cec8a..60285265986 100644 --- a/go/vt/schemadiff/schema_diff.go +++ b/go/vt/schemadiff/schema_diff.go @@ -321,19 +321,40 @@ func (d *SchemaDiff) OrderedDiffs(ctx context.Context) ([]EntityDiff, error) { // We want to apply the changes one by one, and validate the schema after each change for i := range permutatedDiffs { // apply inline - if err := permutationSchema.apply(permutatedDiffs[i:i+1], hints); err != nil { + applyHints := hints + if hints.ForeignKeyCheckStrategy == ForeignKeyCheckStrategyCreateTableFirst { + // This is a strategy that handles foreign key loops in a heuristic way. + // It means: we allow for the very first change to be a CREATE TABLE, and ignore + // any dependencies that CREATE TABLE may have. But then, we require the rest of + // changes to have a strict order. + overrideHints := *hints + overrideHints.ForeignKeyCheckStrategy = ForeignKeyCheckStrategyStrict + if i == 0 { + if _, ok := permutatedDiffs[i].(*CreateTableEntityDiff); ok { + overrideHints.ForeignKeyCheckStrategy = ForeignKeyCheckStrategyIgnore + } + } + applyHints = &overrideHints + } + if err := permutationSchema.apply(permutatedDiffs[i:i+1], applyHints); err != nil { // permutation is invalid return false // continue searching } } + // Good news, we managed to apply all of the permutations! orderedDiffs = append(orderedDiffs, permutatedDiffs...) lastGoodSchema = permutationSchema return true // early break! No need to keep searching }) } - tryPermutateDiffsAcrossHints := func() (found bool, err error) { - for _, fkStrategy := range []int{ForeignKeyCheckStrategyStrict, ForeignKeyCheckStrategyIgnore} { + // We prefer stricter strategy, because that gives best chance of finding a valid path. + // So on best effort basis, we try to find a valid path starting with the strictest strategy, easing + // to more relaxed ones, but never below the preconfigured. + // For example, if the preconfigured strategy is "strict", we will try "strict" and then stop. + // If the preconfigured strategy is "create-table-first", we will try "strict", then "create-table-first", then stop. + tryPermutateDiffsAcrossStrategies := func() (found bool, err error) { + for _, fkStrategy := range []int{ForeignKeyCheckStrategyStrict, ForeignKeyCheckStrategyCreateTableFirst, ForeignKeyCheckStrategyIgnore} { hints := *d.hints hints.ForeignKeyCheckStrategy = fkStrategy found, err = tryPermutateDiffs(&hints) @@ -342,21 +363,20 @@ func (d *SchemaDiff) OrderedDiffs(ctx context.Context) ([]EntityDiff, error) { return found, err } if err != nil { + // No luck with this strategy, let's try the next one. continue } if found { + // Good news! return true, nil } } return found, err } - foundValidPathForClass, err := tryPermutateDiffsAcrossHints() + foundValidPathForClass, err := tryPermutateDiffsAcrossStrategies() if err != nil { return nil, err } - // We prefer stricter strategy, because that gives best chance of finding a valid path. - // So on best effort basis, we try to find a valid path starting with the strictest strategy, easing - // to more relaxed ones, but never below the preconfigured if !foundValidPathForClass { // In this equivalence class, there is no valid permutation. We cannot linearize the diffs. return nil, &ImpossibleApplyDiffOrderError{ diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go index 28e81dd4a84..ab8d56c04d1 100644 --- a/go/vt/schemadiff/schema_diff_test.go +++ b/go/vt/schemadiff/schema_diff_test.go @@ -745,6 +745,74 @@ func TestSchemaDiff(t *testing.T) { instantCapability: InstantDDLCapabilityImpossible, fkStrategy: ForeignKeyCheckStrategyIgnore, }, + { + name: "two table cycle with create, existing column, ignore", + fromQueries: append(createQueries, + "create table t12 (id int primary key, i int, key (i));", + ), + toQueries: append( + createQueries, + "create table t11 (id int primary key, i int, constraint f1201 foreign key (i) references t12 (i) on delete set null);", + "create table t12 (id int primary key, i int, key (i), constraint f1101 foreign key (i) references t11 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + entityOrder: []string{"t11", "t12"}, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyIgnore, + }, + { + name: "two table cycle with create, existing column, changed lexicographic order, ignore", + fromQueries: append(createQueries, + "create table t11 (id int primary key, i int, key (i));", + ), + toQueries: append( + createQueries, + "create table t12 (id int primary key, i int, constraint f1201 foreign key (i) references t11 (i) on delete set null);", + "create table t11 (id int primary key, i int, key (i), constraint f1101 foreign key (i) references t12 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + entityOrder: []string{"t12", "t11"}, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyIgnore, + }, + { + name: "two table cycle with create, create table first", + fromQueries: append(createQueries, + "create table t12 (id int primary key, i int, key (i));", + ), + toQueries: append( + createQueries, + "create table t11 (id int primary key, i int, constraint f1201 foreign key (i) references t12 (i) on delete set null);", + "create table t12 (id int primary key, i int, key (i), constraint f1101 foreign key (i) references t11 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + entityOrder: []string{"t11", "t12"}, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyCreateTableFirst, + }, + { + name: "two table cycle with create, changed lexicographic order, create table first", + fromQueries: append(createQueries, + "create table t11 (id int primary key, i int, key (i));", + ), + toQueries: append( + createQueries, + "create table t12 (id int primary key, i int, constraint f1201 foreign key (i) references t11 (i) on delete set null);", + "create table t11 (id int primary key, i int, key (i), constraint f1101 foreign key (i) references t12 (id) on delete restrict);", + ), + expectDiffs: 2, + expectDeps: 2, + entityOrder: []string{"t12", "t11"}, + sequential: true, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyCreateTableFirst, + }, { name: "add FK", toQueries: []string{ @@ -1063,6 +1131,23 @@ func TestSchemaDiff(t *testing.T) { instantCapability: InstantDDLCapabilityImpossible, fkStrategy: ForeignKeyCheckStrategyStrict, }, + { + name: "add and drop FK, add and drop respective tables, fk create table first", + fromQueries: []string{ + "create table t1 (id int primary key, p int, key p_idx (p));", + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t1 (p) on delete no action);", + }, + toQueries: []string{ + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t3 (p) on delete no action);", + "create table t3 (id int primary key, p int, key p_idx (p));", + }, + expectDiffs: 3, + expectDeps: 2, // [alter t2]/[drop t1], [alter t2]/[create t3] + sequential: true, + entityOrder: []string{"t3", "t2", "t1"}, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyCreateTableFirst, + }, { name: "add and drop FK, add and drop respective tables, fk ignore", fromQueries: []string{ diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go index 2049387140c..cb6bc09df85 100644 --- a/go/vt/schemadiff/types.go +++ b/go/vt/schemadiff/types.go @@ -132,6 +132,7 @@ const ( const ( ForeignKeyCheckStrategyStrict int = iota + ForeignKeyCheckStrategyCreateTableFirst ForeignKeyCheckStrategyIgnore ) From 0da285a96cbea2af33188867f65f33c95cedca10 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 9 Jun 2024 08:55:03 +0300 Subject: [PATCH 7/7] adding more tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/schema_diff_test.go | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/go/vt/schemadiff/schema_diff_test.go b/go/vt/schemadiff/schema_diff_test.go index b1e253b9b27..8adc4fc8d68 100644 --- a/go/vt/schemadiff/schema_diff_test.go +++ b/go/vt/schemadiff/schema_diff_test.go @@ -1116,6 +1116,40 @@ func TestSchemaDiff(t *testing.T) { conflictingDiffs: 2, instantCapability: InstantDDLCapabilityImpossible, }, + { + name: "add and drop FK, add and drop column, impossible order even with create table first strategy", + fromQueries: []string{ + "create table t1 (id int primary key, p int, key p_idx (p));", + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t1 (p) on delete no action);", + }, + toQueries: []string{ + "create table t1 (id int primary key, q int, key q_idx (q));", + "create table t2 (id int primary key, q int, key q_idx (q), foreign key (q) references t1 (q) on delete no action);", + }, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + conflictingDiffs: 2, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyCreateTableFirst, + }, + { + name: "add and drop FK, add and drop column, impossible order even with ignore strategy", + fromQueries: []string{ + "create table t1 (id int primary key, p int, key p_idx (p));", + "create table t2 (id int primary key, p int, key p_idx (p), foreign key (p) references t1 (p) on delete no action);", + }, + toQueries: []string{ + "create table t1 (id int primary key, q int, key q_idx (q));", + "create table t2 (id int primary key, q int, key q_idx (q), foreign key (q) references t1 (q) on delete no action);", + }, + expectDiffs: 2, + expectDeps: 1, + sequential: true, + conflictingDiffs: 2, + instantCapability: InstantDDLCapabilityImpossible, + fkStrategy: ForeignKeyCheckStrategyIgnore, + }, { name: "add and drop FK, add and drop respective tables, fk strict", fromQueries: []string{