Merge branch 'master' into fClose-Fix

README.md

@@ -94,7 +94,7 @@ Please see [Coding gh-ost](doc/coding-ghost.md) for a guide to getting started d
 
 [Download latest release here](https://github.com/github/gh-ost/releases/latest)
 
-`gh-ost` is a Go project; it is built with Go `1.8` (though `1.7` should work as well). To build on your own, use either:
+`gh-ost` is a Go project; it is built with Go `1.9` and above. To build on your own, use either:
 - [script/build](https://github.com/github/gh-ost/blob/master/script/build) - this is the same build script used by CI hence the authoritative; artifact is `./bin/gh-ost` binary.
 - [build.sh](https://github.com/github/gh-ost/blob/master/build.sh) for building `tar.gz` artifacts in `/tmp/gh-ost`
 

build.sh

@@ -10,7 +10,7 @@ function build {
     GOOS=$3
     GOARCH=$4
 
-    if [[ $(go version | egrep "go1[.][012345678]") ]]; then
+    if ! go version | egrep -q 'go(1[.]9|1[.]1[0-9])' ; then
       echo "go version is too low. Must use 1.9 or above"
       exit 1
     fi

go/base/context.go

@@ -28,23 +28,23 @@ type RowsEstimateMethod string
 
 const (
 	TableStatusRowsEstimate RowsEstimateMethod = "TableStatusRowsEstimate"
-	ExplainRowsEstimate                        = "ExplainRowsEstimate"
-	CountRowsEstimate                          = "CountRowsEstimate"
+	ExplainRowsEstimate     RowsEstimateMethod = "ExplainRowsEstimate"
+	CountRowsEstimate       RowsEstimateMethod = "CountRowsEstimate"
 )
 
 type CutOver int
 
 const (
 	CutOverAtomic CutOver = iota
-	CutOverTwoStep         = iota
+	CutOverTwoStep
 )
 
 type ThrottleReasonHint string
 
 const (
 	NoThrottleReasonHint                 ThrottleReasonHint = "NoThrottleReasonHint"
-	UserCommandThrottleReasonHint                           = "UserCommandThrottleReasonHint"
-	LeavingHibernationThrottleReasonHint                    = "LeavingHibernationThrottleReasonHint"
+	UserCommandThrottleReasonHint        ThrottleReasonHint = "UserCommandThrottleReasonHint"
+	LeavingHibernationThrottleReasonHint ThrottleReasonHint = "LeavingHibernationThrottleReasonHint"
 )
 
 const (

go/binlog/binlog_dml_event.go

@@ -7,17 +7,18 @@ package binlog
 
 import (
 	"fmt"
-	"github.com/github/gh-ost/go/sql"
 	"strings"
+
+	"github.com/github/gh-ost/go/sql"
 )
 
 type EventDML string
 
 const (
 	NotDML    EventDML = "NoDML"
-	InsertDML          = "Insert"
-	UpdateDML          = "Update"
-	DeleteDML          = "Delete"
+	InsertDML EventDML = "Insert"
+	UpdateDML EventDML = "Update"
+	DeleteDML EventDML = "Delete"
 )
 
 func ToEventDML(description string) EventDML {

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/builder.go

@@ -15,11 +15,11 @@ type ValueComparisonSign string
 
 const (
 	LessThanComparisonSign            ValueComparisonSign = "<"
-	LessThanOrEqualsComparisonSign                        = "<="
-	EqualsComparisonSign                                  = "="
-	GreaterThanOrEqualsComparisonSign                     = ">="
-	GreaterThanComparisonSign                             = ">"
-	NotEqualsComparisonSign                               = "!="
+	LessThanOrEqualsComparisonSign    ValueComparisonSign = "<="
+	EqualsComparisonSign              ValueComparisonSign = "="
+	GreaterThanOrEqualsComparisonSign ValueComparisonSign = ">="
+	GreaterThanComparisonSign         ValueComparisonSign = ">"
+	NotEqualsComparisonSign           ValueComparisonSign = "!="
 )
 
 // EscapeName will escape a db/table/column/... name by wrapping with backticks.

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)
+	}
+}

go/sql/types.go

@@ -16,12 +16,12 @@ type ColumnType int
 
 const (
 	UnknownColumnType ColumnType = iota
-	TimestampColumnType            = iota
-	DateTimeColumnType             = iota
-	EnumColumnType                 = iota
-	MediumIntColumnType            = iota
-	JSONColumnType                 = iota
-	FloatColumnType                = iota
+	TimestampColumnType
+	DateTimeColumnType
+	EnumColumnType
+	MediumIntColumnType
+	JSONColumnType
+	FloatColumnType
 )
 
 const maxMediumintUnsigned int32 = 16777215

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"