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.

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

schemadiff: SubsequentDiffStrategy: allow/reject multiple changes on same entity #15675

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions go/vt/schemadiff/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,3 +453,18 @@ type UnknownColumnCollationCharsetError struct {
func (e *UnknownColumnCollationCharsetError) Error() string {
return fmt.Sprintf("unable to determine charset for column %s with collation %q", sqlescape.EscapeID(e.Column), e.Collation)
}

type SubsequentDiffRejectedError struct {
Table string
Diffs []EntityDiff
}

func (e *SubsequentDiffRejectedError) Error() string {
var b strings.Builder
b.WriteString(fmt.Sprintf("multiple changes not allowed on table %s. Found:", sqlescape.EscapeID(e.Table)))
for _, d := range e.Diffs {
b.WriteString("\n")
b.WriteString(d.CanonicalStatementString())
}
return b.String()
}
6 changes: 6 additions & 0 deletions go/vt/schemadiff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,12 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints
}
sortAlterOptions(parentAlterTableEntityDiff)

if hints.SubsequentDiffStrategy == SubsequentDiffStrategyReject {
if allSubsequent := AllSubsequent(parentAlterTableEntityDiff); len(allSubsequent) > 1 {
return nil, &SubsequentDiffRejectedError{Table: c.Name(), Diffs: allSubsequent}
}
}

return parentAlterTableEntityDiff, nil
}

Expand Down
39 changes: 39 additions & 0 deletions go/vt/schemadiff/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func TestCreateTableDiff(t *testing.T) {
charset int
algorithm int
enumreorder int
subsequent int
textdiffs []string
}{
{
Expand Down Expand Up @@ -803,6 +804,13 @@ func TestCreateTableDiff(t *testing.T) {
"+ FULLTEXT KEY `name2_ft` (`name2`)",
},
},
{
name: "add two fulltext keys, distinct statements, reject",
from: "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null)",
to: "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null, fulltext key name1_ft(name1), fulltext key name2_ft(name2))",
subsequent: SubsequentDiffStrategyReject,
errorMsg: (&SubsequentDiffRejectedError{Table: "t1"}).Error(),
},
{
name: "add two fulltext keys, unify statements",
from: "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null)",
Expand All @@ -815,6 +823,19 @@ func TestCreateTableDiff(t *testing.T) {
"+ FULLTEXT KEY `name2_ft` (`name2`)",
},
},
{
name: "add two fulltext keys, unify statements, no reject",
from: "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null)",
to: "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null, fulltext key name1_ft(name1), fulltext key name2_ft(name2))",
fulltext: FullTextKeyUnifyStatements,
subsequent: SubsequentDiffStrategyReject,
diff: "alter table t1 add fulltext key name1_ft (name1), add fulltext key name2_ft (name2)",
cdiff: "ALTER TABLE `t1` ADD FULLTEXT KEY `name1_ft` (`name1`), ADD FULLTEXT KEY `name2_ft` (`name2`)",
textdiffs: []string{
"+ FULLTEXT KEY `name1_ft` (`name1`)",
"+ FULLTEXT KEY `name2_ft` (`name2`)",
},
},
{
name: "no fulltext diff",
from: "create table t1 (id int primary key, name tinytext not null, fulltext key name_ft(name) with parser ngram)",
Expand Down Expand Up @@ -1358,6 +1379,14 @@ func TestCreateTableDiff(t *testing.T) {
"+ PARTITION `p4` VALUES LESS THAN (40)",
},
},
{
name: "change partitioning range: statements, multiple, reject",
from: "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20), partition p3 values less than (30))",
to: "create table t1 (id int primary key) partition by range (id) (partition p2 values less than (20), partition p3 values less than (30), partition p4 values less than (40))",
rotation: RangeRotationDistinctStatements,
subsequent: SubsequentDiffStrategyReject,
errorMsg: (&SubsequentDiffRejectedError{Table: "t1"}).Error(),
},
{
name: "change partitioning range: mixed with nonpartition changes",
from: "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20), partition p3 values less than (30))",
Expand All @@ -1371,6 +1400,14 @@ func TestCreateTableDiff(t *testing.T) {
"- PARTITION `p2` VALUES LESS THAN (20),",
},
},
{
name: "change partitioning range: mixed with nonpartition changes, reject",
from: "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20), partition p3 values less than (30))",
to: "create table t1 (id int primary key, i int) partition by range (id) (partition p3 values less than (30))",
rotation: RangeRotationDistinctStatements,
subsequent: SubsequentDiffStrategyReject,
errorMsg: (&SubsequentDiffRejectedError{Table: "t1"}).Error(),
},
{
name: "change partitioning range: single partition change, mixed with nonpartition changes",
from: "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20))",
Expand Down Expand Up @@ -1953,6 +1990,7 @@ func TestCreateTableDiff(t *testing.T) {
hints.TableCharsetCollateStrategy = ts.charset
hints.AlterTableAlgorithmStrategy = ts.algorithm
hints.EnumReorderStrategy = ts.enumreorder
hints.SubsequentDiffStrategy = ts.subsequent
alter, err := c.Diff(other, &hints)

require.Equal(t, len(ts.diffs), len(ts.cdiffs))
Expand Down Expand Up @@ -2367,6 +2405,7 @@ func TestValidate(t *testing.T) {
alter: "alter table t add column i int",
expectErr: &ApplyDuplicatePartitionError{Table: "t1", Partition: "p2"},
},
// More columns and indexes
{
name: "change to visible with alter column",
from: "create table t (id int, i int invisible, primary key (id))",
Expand Down
6 changes: 6 additions & 0 deletions go/vt/schemadiff/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ const (
ForeignKeyCheckStrategyIgnore
)

const (
SubsequentDiffStrategyAllow int = iota
SubsequentDiffStrategyReject
)

// DiffHints is an assortment of rules for diffing entities
type DiffHints struct {
StrictIndexOrdering bool
Expand All @@ -148,6 +153,7 @@ type DiffHints struct {
AlterTableAlgorithmStrategy int
EnumReorderStrategy int
ForeignKeyCheckStrategy int
SubsequentDiffStrategy int
}

func EmptyDiffHints() *DiffHints {
Expand Down
Loading