Improve parser and schemadiff support for ALTER TABLE (#10313)

* Improve parser and schemadiff support for ALTER TABLE

The parser logic for ALTER TABLE was not allowing for the INSTANT
algorithm. This doesn't implement anything for instant DDL, but it
specifically only allows parsing it.

Additionally, the new VISIBLE / INVISIBLE property on columns can be set
using an `ALTER TABLE ALTER COLUMN`, so this is also added to the parser
as well.

Lastly, this makes schemadiff work with such an ALTER TABLE statement.
It doesn't generate these itself at this point, but implementing support
for handling it is simple to add so this was done as well.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

* Add support for ALTER TABLE ALTER INDEX as well

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

go/vt/schemadiff/table.go

@@ -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,
@@ -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.
@@ -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.
@@ -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 {

go/vt/schemadiff/table_test.go

@@ -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",
@@ -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 {

go/vt/sqlparser/ast.go

@@ -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
@@ -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
@@ -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()            {}