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

Improve parser and schemadiff support for ALTER TABLE #10313

Merged
merged 2 commits into from
May 16, 2022
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
87 changes: 84 additions & 3 deletions go/vt/schemadiff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1057,8 +1057,16 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable,
// key exists in both tables
// check diff between before/after columns:
if sqlparser.CanonicalString(t2Key) != sqlparser.CanonicalString(t1Key) {
// keys with same name have different definition. There is no ALTER INDEX statement,
// we're gonna drop and create.
indexVisibilityChange, newVisibility := indexOnlyVisibilityChange(t1Key, t2Key)
if indexVisibilityChange {
alterTable.AlterOptions = append(alterTable.AlterOptions, &sqlparser.AlterIndex{
Name: t2Key.Info.Name,
Invisible: newVisibility,
})
continue
}

// For other changes, we're gonna drop and create.
dropKey := dropKeyStatement(t1Key.Info.Name)
addKey := &sqlparser.AddIndexDefinition{
IndexDefinition: t2Key,
Expand All @@ -1076,6 +1084,37 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable,
}
}

// indexOnlyVisibilityChange checks whether the change on an index is only
// a visibility change. In that case we can use `ALTER INDEX`.
// Returns if this is a visibility only change and if true, whether
// the new visibility is invisible or not.
func indexOnlyVisibilityChange(t1Key, t2Key *sqlparser.IndexDefinition) (bool, bool) {
t1KeyCopy := sqlparser.CloneRefOfIndexDefinition(t1Key)
t2KeyCopy := sqlparser.CloneRefOfIndexDefinition(t2Key)
t1KeyKeptOptions := make([]*sqlparser.IndexOption, 0, len(t1KeyCopy.Options))
t2KeyInvisible := false
for _, opt := range t1KeyCopy.Options {
if strings.EqualFold(opt.Name, "INVISIBLE") {
continue
}
t1KeyKeptOptions = append(t1KeyKeptOptions, opt)
}
t1KeyCopy.Options = t1KeyKeptOptions
t2KeyKeptOptions := make([]*sqlparser.IndexOption, 0, len(t2KeyCopy.Options))
for _, opt := range t2KeyCopy.Options {
if strings.EqualFold(opt.Name, "INVISIBLE") {
t2KeyInvisible = true
continue
}
t2KeyKeptOptions = append(t2KeyKeptOptions, opt)
}
t2KeyCopy.Options = t2KeyKeptOptions
if sqlparser.CanonicalString(t2KeyCopy) == sqlparser.CanonicalString(t1KeyCopy) {
return true, t2KeyInvisible
}
return false, false
}

// evaluateColumnReordering produces a minimal reordering set of columns. To elaborate:
// The function receives two sets of columns. the two must be permutations of one another. Specifically,
// these are the columns shared between the from&to tables.
Expand Down Expand Up @@ -1172,7 +1211,7 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable,
// check diff between before/after columns:
modifyColumnDiff := t1ColEntity.ColumnDiff(t2ColEntity, hints)
if modifyColumnDiff == nil {
// even if there's no apparent change, there can still be implciit changes
// even if there's no apparent change, there can still be implicit changes
// it is possible that the table charset is changed. the column may be some col1 TEXT NOT NULL, possibly in both varsions 1 and 2,
// but implicitly the column has changed its characters set. So we need to explicitly ass a MODIFY COLUMN statement, so that
// MySQL rebuilds it.
Expand Down Expand Up @@ -1492,6 +1531,48 @@ func (c *CreateTableEntity) apply(diff *AlterTableEntityDiff) error {
if !found {
return errors.Wrap(ErrApplyColumnNotFound, opt.NewColDefinition.Name.String())
}
case *sqlparser.AlterColumn:
// we expect the column to exist
found := false
for _, col := range c.TableSpec.Columns {
if col.Name.String() == opt.Column.Name.String() {
found = true
if opt.DropDefault {
col.Type.Options.Default = nil
} else if opt.DefaultVal != nil {
col.Type.Options.Default = opt.DefaultVal
}
col.Type.Options.Invisible = opt.Invisible
break
}
}
if !found {
return errors.Wrap(ErrApplyColumnNotFound, opt.Column.Name.String())
}
case *sqlparser.AlterIndex:
// we expect the index to exist
found := false
for _, idx := range c.TableSpec.Indexes {
if idx.Info.Name.String() == opt.Name.String() {
found = true
if opt.Invisible {
idx.Options = append(idx.Options, &sqlparser.IndexOption{Name: "INVISIBLE"})
} else {
keptOptions := make([]*sqlparser.IndexOption, 0, len(idx.Options))
for _, idxOpt := range idx.Options {
if strings.EqualFold(idxOpt.Name, "INVISIBLE") {
continue
}
keptOptions = append(keptOptions, idxOpt)
}
idx.Options = keptOptions
}
break
}
}
if !found {
return errors.Wrap(ErrApplyKeyNotFound, opt.Name.String())
}
case sqlparser.TableOptions:
// Apply table options. Options that have their DEFAULT value are actually remvoed.
for _, option := range opt {
Expand Down
50 changes: 50 additions & 0 deletions go/vt/schemadiff/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,20 @@ func TestCreateTableDiff(t *testing.T) {
diff: "alter table t1 add key i_idx3 (id)",
cdiff: "ALTER TABLE `t1` ADD KEY `i_idx3` (`id`)",
},
{
name: "key made visible",
from: "create table t1 (`id` int primary key, i int, key i_idx(i) invisible)",
to: "create table t1 (`id` int primary key, i int, key i_idx(i))",
diff: "alter table t1 alter index i_idx visible",
cdiff: "ALTER TABLE `t1` ALTER INDEX `i_idx` VISIBLE",
},
{
name: "key made invisible",
from: "create table t1 (`id` int primary key, i int, key i_idx(i))",
to: "create table t1 (`id` int primary key, i int, key i_idx(i) invisible)",
diff: "alter table t1 alter index i_idx invisible",
cdiff: "ALTER TABLE `t1` ALTER INDEX `i_idx` INVISIBLE",
},
// foreign keys
{
name: "drop foreign key",
Expand Down Expand Up @@ -1041,6 +1055,42 @@ func TestValidate(t *testing.T) {
alter: "alter table t add column i int",
expectErr: ErrApplyDuplicatePartition,
},
{
name: "change to visible with alter column",
from: "create table t (id int, i int invisible, primary key (id))",
alter: "alter table t alter column i set visible",
to: "create table t (id int, i int, primary key (id))",
},
{
name: "change to invisible with alter column",
from: "create table t (id int, i int, primary key (id))",
alter: "alter table t alter column i set invisible",
to: "create table t (id int, i int invisible, primary key (id))",
},
{
name: "remove default with alter column",
from: "create table t (id int, i int default 0, primary key (id))",
alter: "alter table t alter column i drop default",
to: "create table t (id int, i int, primary key (id))",
},
{
name: "change default with alter column",
from: "create table t (id int, i int, primary key (id))",
alter: "alter table t alter column i set default 0",
to: "create table t (id int, i int default 0, primary key (id))",
},
{
name: "change to visible with alter index",
from: "create table t (id int primary key, i int, key i_idx(i) invisible)",
alter: "alter table t alter index i_idx visible",
to: "create table t (id int primary key, i int, key i_idx(i))",
},
{
name: "change to invisible with alter index",
from: "create table t (id int primary key, i int, key i_idx(i))",
alter: "alter table t alter index i_idx invisible",
to: "create table t (id int primary key, i int, key i_idx(i) invisible)",
},
}
hints := DiffHints{}
for _, ts := range tt {
Expand Down
10 changes: 9 additions & 1 deletion go/vt/sqlparser/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,12 @@ type (
// AlgorithmValue is the algorithm specified in the alter table command
AlgorithmValue string

// AlterColumn is used to add or drop defaults to columns in alter table command
// AlterColumn is used to add or drop defaults & visibility to columns in alter table command
AlterColumn struct {
Column *ColName
DropDefault bool
DefaultVal Expr
Invisible *bool
}

// With contains the lists of common table expression and specifies if it is recursive or not
Expand Down Expand Up @@ -174,6 +175,12 @@ type (
Enforced bool
}

// AlterIndex represents the `ALTER INDEX` part in an `ALTER TABLE ALTER INDEX` command.
AlterIndex struct {
Name ColIdent
Invisible bool
}

// KeyState is used to disable or enable the keys in an alter table statement
KeyState struct {
Enable bool
Expand Down Expand Up @@ -733,6 +740,7 @@ func (*AddColumns) iAlterOption() {}
func (AlgorithmValue) iAlterOption() {}
func (*AlterColumn) iAlterOption() {}
func (*AlterCheck) iAlterOption() {}
func (*AlterIndex) iAlterOption() {}
func (*ChangeColumn) iAlterOption() {}
func (*ModifyColumn) iAlterOption() {}
func (*AlterCharset) iAlterOption() {}
Expand Down
33 changes: 24 additions & 9 deletions go/vt/sqlparser/ast_clone.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 37 additions & 12 deletions go/vt/sqlparser/ast_equals.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading