expanding ALTER support

This is a significant, but not complete expansion of the ALTER support in
sqlparser.

It adds support for these ALTER commands:

* `ADD COLUMN`
* `DROP COLUMN`
* `ADD INDEX`
* `DROP INDEX`
* `DROP FOREIGN KEY`
* `DROP PRIMARY KEY`
* `ADD PARTITION`
* `DROP PARTITION`
* `ADD CHECK` (as a no-op; it's parsed but never executed by mysql servers)

The main addition with this API is an additional field on `DDL` that is a slice
of newly created `AlterSpecs`. An `AlterSpec` represents one of the commands
that can occur in the same `ALTER` statement with other commands. This
differentiates them from the `ALTER` statement parse states already in `sql.y`
which are concerned with `ALTER` commands that must be the sole commands in the
statement.

The `ADD PARTITION` and `DROP PARTITION` states are the exception and are new
singleton commands that `sql.y` now supports.

This patch also includes some incidental updates to `go.mod` because of tooling
issues (described in vitessio#5755) and are duplicated in vitessio#5766

Updates vitessio#5705

Signed-off-by: Jeff Hodges <jeff@somethingsimilar.com>

go/vt/sqlparser/ast.go

@@ -171,6 +171,16 @@ type (
 
  // AutoIncSpec is set for AddAutoIncStr.
  AutoIncSpec *AutoIncSpec
+
+ // Ignore is set to the IGNORE option on the ALTER or RENAME statement. It
+ // will end with a trailing space.
+ Ignore string
+
+ // AlterSpecs includes the specifics of an ALTER statement that involves
+ // adding, dropping, or renaming columns or changes a table's
+ // configuration. ALTERs that rename a table are still handled as a RENAME
+ // Action with From and ToTables set.
+ AlterSpecs []*AlterSpec
  }
 
  // ParenSelect is a parenthesized SELECT statement.
@@ -212,6 +222,30 @@ type (
  // It should be used only as an indicator. It does not contain
  // the full AST for the statement.
  OtherAdmin struct{}
+
+ // AlterSpec represents a specific command in an ALTER statement. An ALTER
+ // statement can have some kinds of commands multiple times or with other
+ // commands. Those are the kinds of commands that an AlterSpec represents.
+ AlterSpec struct {
+ // The kind of operation the ALTER command is.
+ Action AlterAction
+ // ColumnsAdded is set when the Action is AlterAddColumn.
+ ColumnsAdded []*ColumnDefinition
+ // ColumnDropped is set when the Action is AlterDropColumn.
+ ColumnDropped ColIdent
+ // IndexAdded is set when the Action is AlterAddIndexOrKey.
+ IndexAdded *IndexDefinition
+ // IndexDropped is set when the Action is AlterDropIndexOrKey. It's the name of the index or key dropped by this ALTER command.
+ IndexDropped *ColIdent
+ // PartitionAdded is set when the Action is AlterAddPartition.
+ PartitionAdded *PartitionDefinition
+ // PartitionsDropped is the name of the partition is dropped. It's set when
+ // AlterDropPartition is set.
+ PartitionsDropped Partitions
+
+ // ForeignKeyDropped is set when the Action is AlterDropForeignKey.
+ ForeignKeyDropped *ColIdent
+ }
 )
 
 func (*Union) iStatement() {}
@@ -323,11 +357,13 @@ type IndexDefinition struct {
 
 // IndexInfo describes the name and type of an index in a CREATE TABLE statement
 type IndexInfo struct {
- Type string
- Name ColIdent
- Primary bool
- Spatial bool
- Unique bool
+ Type string
+ Name ColIdent
+ Primary bool
+ Spatial bool
+ Foreign bool
+ Fulltext bool
+ Unique bool
 }
 
 // VindexSpec defines a vindex for a CREATE VINDEX or DROP VINDEX statement
@@ -907,7 +943,23 @@ func (node *DDL) Format(buf *TrackedBuffer) {
  buf.Myprintf(", %v to %v", node.FromTables[i], node.ToTables[i])
  }
  case AlterStr:
- if node.PartitionSpec != nil {
+ if len(node.AlterSpecs) != 0 {
+ buf.Myprintf("%s %stable %v ", node.Action, node.Ignore, node.Table)
+ if len(node.AlterSpecs) == 1 {
+ buf.Myprintf("%v", node.AlterSpecs[0])
+ } else {
+ buf.Myprintf("(")
+ for i, spec := range node.AlterSpecs {
+ if i != 0 {
+ buf.Myprintf(", %v", spec)
+ } else {
+ buf.Myprintf("%v", spec)
+ }
+ }
+ buf.Myprintf(")")
+ }
+
+ } else if node.PartitionSpec != nil {
  buf.Myprintf("%s table %v %v", node.Action, node.Table, node.PartitionSpec)
  } else {
  buf.Myprintf("%s table %v", node.Action, node.Table)
@@ -996,6 +1048,35 @@ func (ts *TableSpec) Format(buf *TrackedBuffer) {
  buf.Myprintf("\n)%s", strings.Replace(ts.Options, ", ", ",\n ", -1))
 }
 
+// Format formats the node as a SQL command.
+func (spec *AlterSpec) Format(buf *TrackedBuffer) {
+ switch spec.Action {
+ case AlterAddColumn:
+ buf.Myprintf("add column ")
+ for i, col := range spec.ColumnsAdded {
+ if i == 0 {
+ buf.Myprintf("%v", col)
+ } else {
+ buf.Myprintf(", %v", col)
+ }
+ }
+ case AlterDropColumn:
+ buf.Myprintf("drop column %v", spec.ColumnDropped)
+ case AlterAddIndexOrKey:
+ buf.Myprintf("add %v", spec.IndexAdded)
+ case AlterDropIndexOrKey:
+ buf.Myprintf("drop index %v", *spec.IndexDropped)
+ case AlterAddPartition:
+ buf.Myprintf("add partition %v", spec.PartitionAdded)
+ case AlterDropPartition:
+ buf.Myprintf("drop%v", spec.PartitionsDropped)
+ case AlterDropForeignKey:
+ buf.Myprintf("drop foreign key %v", spec.ForeignKeyDropped)
+ case AlterDropPrimaryKey:
+ buf.Myprintf("drop primary key")
+ }
+}
+
 // Format formats the node.
 func (col *ColumnDefinition) Format(buf *TrackedBuffer) {
  buf.Myprintf("%v %v", col.Name, &col.Type)

go/vt/sqlparser/constants.go

@@ -161,3 +161,35 @@ const (
  TxReadOnly = "read only"
  TxReadWrite = "read write"
 )
+
+// AlterAction is used to determine what kind of ALTER command is being
+// represented by an AlterSpec.
+type AlterAction int
+
+const (
+ // AlterAddColumn is set when the ALTER command adds a column. Multiple
+ // columns can be added within one `ADD COLUMN` sub-command.
+ AlterAddColumn AlterAction = iota
+
+ // AlterDropColumn is set when the ALTER command drops a column or index.
+ AlterDropColumn
+
+ // AlterAddIndexOrKey is set when the ALTER command adds an index or key.
+ AlterAddIndexOrKey
+
+ // AlterDropIndexOrKey is set when the ALTER command drops an index or key.
+ AlterDropIndexOrKey
+
+ // AlterAddPartition is set when the ALTER command adds a partition.
+ AlterAddPartition
+
+ // AlterDropPartition is set when the ALTER command drops a partition.
+ AlterDropPartition
+
+ // AlterDropForeignKey is set when the ALTER command drops a foreign key constraint.
+ AlterDropForeignKey
+
+ // AlterDropPrimaryKey is set when the ALTER command drops the primary key
+ // from the table.
+ AlterDropPrimaryKey
+)

go/vt/sqlparser/parse_test.go

@@ -25,6 +25,8 @@ import (
  "strings"
  "sync"
  "testing"
+
+ "github.com/stretchr/testify/require"
 )
 
 var (
@@ -824,20 +826,20 @@ var (
  }, {
  input: "set sql_safe_updates = 1",
  }, {
- input: "alter ignore table a add foo",
- output: "alter table a",
+ input: "alter ignore table a add foo bigint",
+ output: "alter ignore table a add column foo bigint",
  }, {
- input: "alter table a add foo",
- output: "alter table a",
+ input: "alter table a add foo bigint",
+ output: "alter table a add column foo bigint",
  }, {
  input: "alter table a add spatial key foo (column1)",
- output: "alter table a",
+ output: "alter table a add spatial key foo (column1)",
  }, {
  input: "alter table a add unique key foo (column1)",
- output: "alter table a",
+ output: "alter table a add unique key foo (column1)",
  }, {
- input: "alter table `By` add foo",
- output: "alter table `By`",
+ input: "alter table `By` add foo bigint",
+ output: "alter table `By` add column foo bigint",
  }, {
  input: "alter table a alter foo",
  output: "alter table a",
@@ -849,7 +851,7 @@ var (
  output: "alter table a",
  }, {
  input: "alter table a drop foo",
- output: "alter table a",
+ output: "alter table a drop column foo",
  }, {
  input: "alter table a disable foo",
  output: "alter table a",
@@ -906,61 +908,54 @@ var (
  output: "alter table a",
  }, {
  input: "alter table a add column id int",
- output: "alter table a",
+ output: "alter table a add column id int",
  }, {
  input: "alter table a add index idx (id)",
- output: "alter table a",
+ output: "alter table a add index idx (id)",
  }, {
  input: "alter table a add fulltext index idx (id)",
- output: "alter table a",
+ output: "alter table a add fulltext index idx (id)",
  }, {
  input: "alter table a add spatial index idx (id)",
- output: "alter table a",
+ output: "alter table a add spatial index idx (id)",
  }, {
  input: "alter table a add foreign key",
  output: "alter table a",
  }, {
- input: "alter table a add primary key",
+ input: "alter table a add primary key idx (id)",
  output: "alter table a",
  }, {
  input: "alter table a add constraint",
  output: "alter table a",
  }, {
- input: "alter table a add id",
- output: "alter table a",
+ input: "alter table a add id int",
+ output: "alter table a add column id int",
  }, {
- input: "alter table a drop column id int",
- output: "alter table a",
+ input: "alter table a drop column id",
+ output: "alter table a drop column id",
  }, {
  input: "alter table a drop partition p2712",
- output: "alter table a",
+ output: "alter table a drop partition (p2712)",
  }, {
- input: "alter table a drop index idx (id)",
- output: "alter table a",
+ input: "alter table a drop index idx",
+ output: "alter table a drop index idx",
  }, {
- input: "alter table a drop fulltext index idx (id)",
+ // ADD CHECK is parsed but ignored by mysql 5.7 and later, so we're
+ // skipping it.
+ input: "alter table a add check ch_1 (1 = 1)",
  output: "alter table a",
  }, {
- input: "alter table a drop spatial index idx (id)",
- output: "alter table a",
- }, {
- input: "alter table a add check ch_1",
- output: "alter table a",
- }, {
- input: "alter table a drop check ch_1",
- output: "alter table a",
- }, {
- input: "alter table a drop foreign key",
- output: "alter table a",
+ input: "alter table a drop foreign key fk_idx",
+ output: "alter table a drop foreign key fk_idx",
  }, {
  input: "alter table a drop primary key",
- output: "alter table a",
+ output: "alter table a drop primary key",
  }, {
  input: "alter table a drop constraint",
  output: "alter table a",
  }, {
  input: "alter table a drop id",
- output: "alter table a",
+ output: "alter table a drop column id",
  }, {
  input: "alter database d default character set = charset",
  output: "alter database d",
@@ -1505,26 +1500,23 @@ var (
 )
 
 func TestValid(t *testing.T) {
- for _, tcase := range validSQL {
- if tcase.output == "" {
- tcase.output = tcase.input
- }
- tree, err := Parse(tcase.input)
- if err != nil {
- t.Errorf("Parse(%q) err: %v, want nil", tcase.input, err)
- continue
- }
- out := String(tree)
- if out != tcase.output {
- t.Errorf("Parse(%q) = %q, want: %q", tcase.input, out, tcase.output)
- }
- // This test just exercises the tree walking functionality.
- // There's no way automated way to verify that a node calls
- // all its children. But we can examine code coverage and
- // ensure that all walkSubtree functions were called.
- Walk(func(node SQLNode) (bool, error) {
- return true, nil
- }, tree)
+ for i, tcase := range validSQL {
+ t.Run(fmt.Sprintf("test-%d", i), func(t *testing.T) {
+ if tcase.output == "" {
+ tcase.output = tcase.input
+ }
+ tree, err := Parse(tcase.input)
+ require.NoError(t, err, "Parse(%q) err: %v, want nil", tcase.input, err)
+ out := String(tree)
+ require.Equal(t, out, tcase.output, "String(Parse(%q)) = %q, want: %q", tcase.input, out, tcase.output)
+ // This test just exercises the tree walking functionality.
+ // There's no way automated way to verify that a node calls
+ // all its children. But we can examine code coverage and
+ // ensure that all walkSubtree functions were called.
+ Walk(func(node SQLNode) (bool, error) {
+ return true, nil
+ }, tree)
+ })
  }
 }