Merge pull request #404 from github/drop-add-same-column

Drop, then add same column

go/base/context.go

@@ -181,6 +181,7 @@ type MigrationContext struct {
 	UniqueKey                        *sql.UniqueKey
 	SharedColumns                    *sql.ColumnList
 	ColumnRenameMap                  map[string]string
+	DroppedColumnsMap                map[string]bool
 	MappedSharedColumns              *sql.ColumnList
 	MigrationRangeMinValues          *sql.ColumnValues
 	MigrationRangeMaxValues          *sql.ColumnValues

go/logic/inspect.go

@@ -662,7 +662,14 @@ func (this *Inspector) getSharedColumns(originalColumns, ghostColumns *sql.Colum
 	}
 	sharedColumnNames := []string{}
 	for _, originalColumn := range originalColumns.Names() {
+		isSharedColumn := false
 		if columnsInGhost[originalColumn] || columnsInGhost[columnRenameMap[originalColumn]] {
+			isSharedColumn = true
+		}
+		if this.migrationContext.DroppedColumnsMap[originalColumn] {
+			isSharedColumn = false
+		}
+		if isSharedColumn {
 			sharedColumnNames = append(sharedColumnNames, originalColumn)
 		}
 	}

go/logic/migrator.go

@@ -248,6 +248,7 @@ func (this *Migrator) validateStatement() (err error) {
 		}
 		log.Infof("Alter statement has column(s) renamed. gh-ost finds the following renames: %v; --approve-renamed-columns is given and so migration proceeds.", this.parser.GetNonTrivialRenames())
 	}
+	this.migrationContext.DroppedColumnsMap = this.parser.DroppedColumnsMap()
 	return nil
 }
 

go/sql/parser.go

@@ -14,15 +14,18 @@ import (
 var (
 	sanitizeQuotesRegexp = regexp.MustCompile("('[^']*')")
 	renameColumnRegexp   = regexp.MustCompile(`(?i)\bchange\s+(column\s+|)([\S]+)\s+([\S]+)\s+`)
+	dropColumnRegexp     = regexp.MustCompile(`(?i)\bdrop\s+(column\s+|)([\S]+)$`)
 )
 
 type Parser struct {
 	columnRenameMap map[string]string
+	droppedColumns  map[string]bool
 }
 
 func NewParser() *Parser {
 	return &Parser{
 		columnRenameMap: make(map[string]string),
+		droppedColumns:  make(map[string]bool),
 	}
 }
 
@@ -59,10 +62,9 @@ func (this *Parser) sanitizeQuotesFromAlterStatement(alterStatement string) (str
 	return strippedStatement
 }
 
-func (this *Parser) ParseAlterStatement(alterStatement string) (err error) {
-	alterTokens, _ := this.tokenizeAlterStatement(alterStatement)
-	for _, alterToken := range alterTokens {
-		alterToken = this.sanitizeQuotesFromAlterStatement(alterToken)
+func (this *Parser) parseAlterToken(alterToken string) (err error) {
+	{
+		// rename
 		allStringSubmatch := renameColumnRegexp.FindAllStringSubmatch(alterToken, -1)
 		for _, submatch := range allStringSubmatch {
 			if unquoted, err := strconv.Unquote(submatch[2]); err == nil {
@@ -71,10 +73,28 @@ func (this *Parser) ParseAlterStatement(alterStatement string) (err error) {
 			if unquoted, err := strconv.Unquote(submatch[3]); err == nil {
 				submatch[3] = unquoted
 			}
-
 			this.columnRenameMap[submatch[2]] = submatch[3]
 		}
 	}
+	{
+		// drop
+		allStringSubmatch := dropColumnRegexp.FindAllStringSubmatch(alterToken, -1)
+		for _, submatch := range allStringSubmatch {
+			if unquoted, err := strconv.Unquote(submatch[2]); err == nil {
+				submatch[2] = unquoted
+			}
+			this.droppedColumns[submatch[2]] = true
+		}
+	}
+	return nil
+}
+
+func (this *Parser) ParseAlterStatement(alterStatement string) (err error) {
+	alterTokens, _ := this.tokenizeAlterStatement(alterStatement)
+	for _, alterToken := range alterTokens {
+		alterToken = this.sanitizeQuotesFromAlterStatement(alterToken)
+		this.parseAlterToken(alterToken)
+	}
 	return nil
 }
 
@@ -91,3 +111,7 @@ func (this *Parser) GetNonTrivialRenames() map[string]string {
 func (this *Parser) HasNonTrivialRenames() bool {
 	return len(this.GetNonTrivialRenames()) > 0
 }
+
+func (this *Parser) DroppedColumnsMap() map[string]bool {
+	return this.droppedColumns
+}

go/sql/parser_test.go

@@ -120,3 +120,42 @@ func TestSanitizeQuotesFromAlterStatement(t *testing.T) {
 		test.S(t).ExpectEquals(strippedStatement, "change column i int ''")
 	}
 }
+
+func TestParseAlterStatementDroppedColumns(t *testing.T) {
+
+	{
+		parser := NewParser()
+		statement := "drop column b"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(len(parser.droppedColumns), 1)
+		test.S(t).ExpectTrue(parser.droppedColumns["b"])
+	}
+	{
+		parser := NewParser()
+		statement := "drop column b, drop key c_idx, drop column `d`"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(len(parser.droppedColumns), 2)
+		test.S(t).ExpectTrue(parser.droppedColumns["b"])
+		test.S(t).ExpectTrue(parser.droppedColumns["d"])
+	}
+	{
+		parser := NewParser()
+		statement := "drop column b, drop key c_idx, drop column `d`, drop `e`, drop primary key, drop foreign key fk_1"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(len(parser.droppedColumns), 3)
+		test.S(t).ExpectTrue(parser.droppedColumns["b"])
+		test.S(t).ExpectTrue(parser.droppedColumns["d"])
+		test.S(t).ExpectTrue(parser.droppedColumns["e"])
+	}
+	{
+		parser := NewParser()
+		statement := "drop column b, drop bad statement, add column i int"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectEquals(len(parser.droppedColumns), 1)
+		test.S(t).ExpectTrue(parser.droppedColumns["b"])
+	}
+}

localtests/drop-null-add-not-null/create.sql

@@ -0,0 +1,30 @@
+drop table if exists gh_ost_test;
+create table gh_ost_test (
+  id int auto_increment,
+  c1 int null,
+  c2 int not null,
+  primary key (id)
+) auto_increment=1;
+
+insert into gh_ost_test values (null, null, 17);
+insert into gh_ost_test values (null, null, 19);
+
+drop event if exists gh_ost_test;
+delimiter ;;
+create event gh_ost_test
+  on schedule every 1 second
+  starts current_timestamp
+  ends current_timestamp + interval 60 second
+  on completion not preserve
+  enable
+  do
+begin
+  insert ignore into gh_ost_test values (101, 11, 23);
+  insert ignore into gh_ost_test values (102, 13, 23);
+  insert into gh_ost_test values (null, 17, 23);
+  insert into gh_ost_test values (null, null, 29);
+  set @last_insert_id := last_insert_id();
+  -- update gh_ost_test set c2=c2+@last_insert_id where id=@last_insert_id order by id desc limit 1;
+  delete from gh_ost_test where id=1;
+  delete from gh_ost_test where c1=13; -- id=2
+end ;;

localtests/drop-null-add-not-null/extra_args

@@ -0,0 +1 @@
+--alter="drop column c1, add column c1 int not null default 47"

localtests/drop-null-add-not-null/ghost_columns

@@ -0,0 +1 @@
+c2

localtests/drop-null-add-not-null/orig_columns

@@ -0,0 +1 @@
+c2