Skip to content

Commit

Permalink
Rejecting RENAME TO|AS
Browse files Browse the repository at this point in the history
  • Loading branch information
Shlomi Noach committed May 6, 2018
1 parent 2b69a5d commit fb00a13
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 0 deletions.
4 changes: 4 additions & 0 deletions go/logic/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions go/sql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
39 changes: 39 additions & 0 deletions go/sql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
22 changes: 22 additions & 0 deletions localtests/fail-rename-table/create.sql
Original file line number Diff line number Diff line change
@@ -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 ;;
1 change: 1 addition & 0 deletions localtests/fail-rename-table/expect_failure
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER statement seems to RENAME the table
1 change: 1 addition & 0 deletions localtests/fail-rename-table/extra_args
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--alter="rename as something_else"

0 comments on commit fb00a13

Please sign in to comment.