From fb00a1387161d2678e280dc55eb380c9a2a8da3a Mon Sep 17 00:00:00 2001
From: Shlomi Noach <shlomi-noach@github.com>
Date: Sun, 6 May 2018 11:19:03 +0300
Subject: [PATCH] Rejecting RENAME TO|AS

---
 go/logic/migrator.go                        |  4 +++
 go/sql/parser.go                            | 12 +++++++
 go/sql/parser_test.go                       | 39 +++++++++++++++++++++
 localtests/fail-rename-table/create.sql     | 22 ++++++++++++
 localtests/fail-rename-table/expect_failure |  1 +
 localtests/fail-rename-table/extra_args     |  1 +
 6 files changed, 79 insertions(+)
 create mode 100644 localtests/fail-rename-table/create.sql
 create mode 100644 localtests/fail-rename-table/expect_failure
 create mode 100644 localtests/fail-rename-table/extra_args

diff --git a/go/logic/migrator.go b/go/logic/migrator.go
index 673a98e..f52e2b8 100644
--- a/go/logic/migrator.go
+++ b/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 {
diff --git a/go/sql/parser.go b/go/sql/parser.go
index 7114e10..447cbcc 100644
--- a/go/sql/parser.go
+++ b/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
+}
diff --git a/go/sql/parser_test.go b/go/sql/parser_test.go
index 3e1d845..5d38130 100644
--- a/go/sql/parser_test.go
+++ b/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)
+	}
+}
diff --git a/localtests/fail-rename-table/create.sql b/localtests/fail-rename-table/create.sql
new file mode 100644
index 0000000..5bb45f2
--- /dev/null
+++ b/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 ;;
diff --git a/localtests/fail-rename-table/expect_failure b/localtests/fail-rename-table/expect_failure
new file mode 100644
index 0000000..e444c17
--- /dev/null
+++ b/localtests/fail-rename-table/expect_failure
@@ -0,0 +1 @@
+ALTER statement seems to RENAME the table
diff --git a/localtests/fail-rename-table/extra_args b/localtests/fail-rename-table/extra_args
new file mode 100644
index 0000000..28a7587
--- /dev/null
+++ b/localtests/fail-rename-table/extra_args
@@ -0,0 +1 @@
+--alter="rename as something_else"