Merge pull request #588 from github/reject-rename-table

Rejecting RENAME TO|AS

go/logic/migrator.go

@@ -255,7 +255,11 @@ func (this *Migrator) listenOnPanicAbort() {
 // validateStatement validates the `alter` statement meets criteria.
 // At this time this means:
 // - column renames are approved
+// - no table rename allowed
 func (this *Migrator) validateStatement() (err error) {
+	if this.parser.IsRenameTable() {
+		return fmt.Errorf("ALTER statement seems to RENAME the table. This is not supported, and you should run your RENAME outside gh-ost.")
+	}
 	if this.parser.HasNonTrivialRenames() && !this.migrationContext.SkipRenamedColumns {
 		this.migrationContext.ColumnRenameMap = this.parser.GetNonTrivialRenames()
 		if !this.migrationContext.ApproveRenamedColumns {

go/sql/parser.go

@@ -15,11 +15,13 @@ 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]+)$`)
+	renameTableRegexp    = regexp.MustCompile(`(?i)\brename\s+(to|as)\s+`)
 )
 
 type Parser struct {
 	columnRenameMap map[string]string
 	droppedColumns  map[string]bool
+	isRenameTable   bool
 }
 
 func NewParser() *Parser {
@@ -86,6 +88,12 @@ func (this *Parser) parseAlterToken(alterToken string) (err error) {
 			this.droppedColumns[submatch[2]] = true
 		}
 	}
+	{
+		// rename table
+		if renameTableRegexp.MatchString(alterToken) {
+			this.isRenameTable = true
+		}
+	}
 	return nil
 }
 
@@ -115,3 +123,7 @@ func (this *Parser) HasNonTrivialRenames() bool {
 func (this *Parser) DroppedColumnsMap() map[string]bool {
 	return this.droppedColumns
 }
+
+func (this *Parser) IsRenameTable() bool {
+	return this.isRenameTable
+}

go/sql/parser_test.go

@@ -159,3 +159,42 @@ func TestParseAlterStatementDroppedColumns(t *testing.T) {
 		test.S(t).ExpectTrue(parser.droppedColumns["b"])
 	}
 }
+
+func TestParseAlterStatementRenameTable(t *testing.T) {
+
+	{
+		parser := NewParser()
+		statement := "drop column b"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectFalse(parser.isRenameTable)
+	}
+	{
+		parser := NewParser()
+		statement := "rename as something_else"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectTrue(parser.isRenameTable)
+	}
+	{
+		parser := NewParser()
+		statement := "drop column b, rename as something_else"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectTrue(parser.isRenameTable)
+	}
+	{
+		parser := NewParser()
+		statement := "engine=innodb rename as something_else"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectTrue(parser.isRenameTable)
+	}
+	{
+		parser := NewParser()
+		statement := "rename as something_else, engine=innodb"
+		err := parser.ParseAlterStatement(statement)
+		test.S(t).ExpectNil(err)
+		test.S(t).ExpectTrue(parser.isRenameTable)
+	}
+}

localtests/fail-rename-table/create.sql

@@ -0,0 +1,22 @@
+drop table if exists gh_ost_test;
+create table gh_ost_test (
+  id int auto_increment,
+  i int not null,
+  ts timestamp,
+  primary key(id)
+) auto_increment=1;
+
+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 into gh_ost_test values (null, 11, now());
+  insert into gh_ost_test values (null, 13, now());
+  insert into gh_ost_test values (null, 17, now());
+end ;;

localtests/fail-rename-table/expect_failure

@@ -0,0 +1 @@
+ALTER statement seems to RENAME the table

localtests/fail-rename-table/extra_args

@@ -0,0 +1 @@
+--alter="rename as something_else"