diff --git a/go/logic/migrator.go b/go/logic/migrator.go index cf59972..8f61425 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -229,7 +229,7 @@ func (this *Migrator) validateStatement() (err error) { if this.parser.HasNonTrivialRenames() && !this.migrationContext.SkipRenamedColumns { this.migrationContext.ColumnRenameMap = this.parser.GetNonTrivialRenames() if !this.migrationContext.ApproveRenamedColumns { - return fmt.Errorf("gh-ost believes the ALTER statement renames columns, as follows: %v; as precation, you are asked to confirm gh-ost is correct, and provide with `--approve-renamed-columns`, and we're all happy. Or you can skip renamed columns via `--skip-renamed-columns`, in which case column data may be lost", this.parser.GetNonTrivialRenames()) + return fmt.Errorf("gh-ost believes the ALTER statement renames columns, as follows: %v; as precaution, you are asked to confirm gh-ost is correct, and provide with `--approve-renamed-columns`, and we're all happy. Or you can skip renamed columns via `--skip-renamed-columns`, in which case column data may be lost", this.parser.GetNonTrivialRenames()) } 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()) } diff --git a/go/sql/parser.go b/go/sql/parser.go index 144f265..b81b31c 100644 --- a/go/sql/parser.go +++ b/go/sql/parser.go @@ -8,10 +8,12 @@ package sql import ( "regexp" "strconv" + "strings" ) var ( - renameColumnRegexp = regexp.MustCompile(`(?i)change\s+(column\s+|)([\S]+)\s+([\S]+)\s+`) + sanitizeQuotesRegexp = regexp.MustCompile("('[^']*')") + renameColumnRegexp = regexp.MustCompile(`(?i)\bchange\s+(column\s+|)([\S]+)\s+([\S]+)\s+`) ) type Parser struct { @@ -24,17 +26,54 @@ func NewParser() *Parser { } } -func (this *Parser) ParseAlterStatement(alterStatement string) (err error) { - allStringSubmatch := renameColumnRegexp.FindAllStringSubmatch(alterStatement, -1) - for _, submatch := range allStringSubmatch { - if unquoted, err := strconv.Unquote(submatch[2]); err == nil { - submatch[2] = unquoted - } - if unquoted, err := strconv.Unquote(submatch[3]); err == nil { - submatch[3] = unquoted +func (this *Parser) tokenizeAlterStatement(alterStatement string) (tokens []string, err error) { + terminatingQuote := rune(0) + f := func(c rune) bool { + switch { + case c == terminatingQuote: + terminatingQuote = rune(0) + return false + case terminatingQuote != rune(0): + return false + case c == '\'': + terminatingQuote = c + return false + case c == '(': + terminatingQuote = ')' + return false + default: + return c == ',' } + } - this.columnRenameMap[submatch[2]] = submatch[3] + tokens = strings.FieldsFunc(alterStatement, f) + for i := range tokens { + tokens[i] = strings.TrimSpace(tokens[i]) + } + return tokens, nil +} + +func (this *Parser) sanitizeQuotesFromAlterStatement(alterStatement string) (strippedStatement string) { + strippedStatement = alterStatement + strippedStatement = sanitizeQuotesRegexp.ReplaceAllString(strippedStatement, "''") + return strippedStatement +} + +func (this *Parser) ParseAlterStatement(alterStatement string) (err error) { + alterTokens, _ := this.tokenizeAlterStatement(alterStatement) + for _, alterToken := range alterTokens { + alterToken = this.sanitizeQuotesFromAlterStatement(alterToken) + allStringSubmatch := renameColumnRegexp.FindAllStringSubmatch(alterToken, -1) + for _, submatch := range allStringSubmatch { + if unquoted, err := strconv.Unquote(submatch[2]); err == nil { + submatch[2] = unquoted + } + if unquoted, err := strconv.Unquote(submatch[3]); err == nil { + submatch[3] = unquoted + } + + this.columnRenameMap[submatch[2]] = submatch[3] + } } return nil } diff --git a/go/sql/parser_test.go b/go/sql/parser_test.go index 2107963..876429a 100644 --- a/go/sql/parser_test.go +++ b/go/sql/parser_test.go @@ -6,6 +6,7 @@ package sql import ( + "reflect" "testing" "github.com/outbrain/golib/log" @@ -66,3 +67,58 @@ func TestParseAlterStatementNonTrivial(t *testing.T) { test.S(t).ExpectEquals(renames["f"], "fl") } } + +func TestTokenizeAlterStatement(t *testing.T) { + parser := NewParser() + { + alterStatement := "add column t int" + tokens, _ := parser.tokenizeAlterStatement(alterStatement) + test.S(t).ExpectTrue(reflect.DeepEqual(tokens, []string{"add column t int"})) + } + { + alterStatement := "add column t int, change column i int" + tokens, _ := parser.tokenizeAlterStatement(alterStatement) + test.S(t).ExpectTrue(reflect.DeepEqual(tokens, []string{"add column t int", "change column i int"})) + } + { + alterStatement := "add column t int, change column i int 'some comment'" + tokens, _ := parser.tokenizeAlterStatement(alterStatement) + test.S(t).ExpectTrue(reflect.DeepEqual(tokens, []string{"add column t int", "change column i int 'some comment'"})) + } + { + alterStatement := "add column t int, change column i int 'some comment, with comma'" + tokens, _ := parser.tokenizeAlterStatement(alterStatement) + test.S(t).ExpectTrue(reflect.DeepEqual(tokens, []string{"add column t int", "change column i int 'some comment, with comma'"})) + } + { + alterStatement := "add column t int, add column d decimal(10,2)" + tokens, _ := parser.tokenizeAlterStatement(alterStatement) + test.S(t).ExpectTrue(reflect.DeepEqual(tokens, []string{"add column t int", "add column d decimal(10,2)"})) + } + { + alterStatement := "add column t int, add column e enum('a','b','c')" + tokens, _ := parser.tokenizeAlterStatement(alterStatement) + log.Errorf("%#v", tokens) + test.S(t).ExpectTrue(reflect.DeepEqual(tokens, []string{"add column t int", "add column e enum('a','b','c')"})) + } + { + alterStatement := "add column t int(11), add column e enum('a','b','c')" + tokens, _ := parser.tokenizeAlterStatement(alterStatement) + log.Errorf("%#v", tokens) + test.S(t).ExpectTrue(reflect.DeepEqual(tokens, []string{"add column t int(11)", "add column e enum('a','b','c')"})) + } +} + +func TestSanitizeQuotesFromAlterStatement(t *testing.T) { + parser := NewParser() + { + alterStatement := "add column e enum('a','b','c')" + strippedStatement := parser.sanitizeQuotesFromAlterStatement(alterStatement) + test.S(t).ExpectEquals(strippedStatement, "add column e enum('','','')") + } + { + alterStatement := "change column i int 'some comment, with comma'" + strippedStatement := parser.sanitizeQuotesFromAlterStatement(alterStatement) + test.S(t).ExpectEquals(strippedStatement, "change column i int ''") + } +} diff --git a/localtests/rename-none-column/create.sql b/localtests/rename-none-column/create.sql new file mode 100644 index 0000000..1db5081 --- /dev/null +++ b/localtests/rename-none-column/create.sql @@ -0,0 +1,8 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + c1 int not null, + primary key (id) +) auto_increment=1; + +drop event if exists gh_ost_test; diff --git a/localtests/rename-none-column/extra_args b/localtests/rename-none-column/extra_args new file mode 100644 index 0000000..889e91a --- /dev/null +++ b/localtests/rename-none-column/extra_args @@ -0,0 +1 @@ +--alter="add column exchange double comment 'exchange rate used for pay in your own currency'" diff --git a/localtests/rename-none-comment/create.sql b/localtests/rename-none-comment/create.sql new file mode 100644 index 0000000..1db5081 --- /dev/null +++ b/localtests/rename-none-comment/create.sql @@ -0,0 +1,8 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + c1 int not null, + primary key (id) +) auto_increment=1; + +drop event if exists gh_ost_test; diff --git a/localtests/rename-none-comment/extra_args b/localtests/rename-none-comment/extra_args new file mode 100644 index 0000000..627b220 --- /dev/null +++ b/localtests/rename-none-comment/extra_args @@ -0,0 +1 @@ +--alter="add column exchange_rate double comment 'change rate used for pay in your own currency'"