From c71dbf9ef3e8511b1eefb3ec38ef6d3ce768f0e6 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 14 May 2021 15:32:56 +0200 Subject: [PATCH] Copy auto increment (#967) * v1.1.0 * WIP: copying AUTO_INCREMENT value to ghost table Initial commit: towards setting up a test suite Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * greping for 'expect_table_structure' content * Adding simple test for 'expect_table_structure' scenario * adding tests for AUTO_INCREMENT value after row deletes. Should initially fail * clear event beforehand * parsing AUTO_INCREMENT from alter query, reading AUTO_INCREMENT from original table, applying AUTO_INCREMENT value onto ghost table if applicable and user has not specified AUTO_INCREMENT in alter statement * support GetUint64 Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * minor update to test Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * adding test for user defined AUTO_INCREMENT statement Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/base/context.go | 1 + go/logic/applier.go | 19 +++++++++++++++ go/logic/inspect.go | 22 +++++++++++++++++ go/logic/migrator.go | 8 +++++++ go/sql/parser.go | 19 ++++++++++++--- go/sql/parser_test.go | 24 +++++++++++++++++++ .../create.sql | 17 +++++++++++++ .../expect_table_structure | 1 + .../extra_args | 1 + localtests/autoinc-copy-deletes/create.sql | 17 +++++++++++++ .../expect_table_structure | 1 + localtests/autoinc-copy-simple/create.sql | 13 ++++++++++ .../expect_table_structure | 1 + localtests/test.sh | 13 ++++++++++ .../outbrain/golib/sqlutils/sqlutils.go | 13 ++++++++++ 15 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 localtests/autoinc-copy-deletes-user-defined/create.sql create mode 100644 localtests/autoinc-copy-deletes-user-defined/expect_table_structure create mode 100644 localtests/autoinc-copy-deletes-user-defined/extra_args create mode 100644 localtests/autoinc-copy-deletes/create.sql create mode 100644 localtests/autoinc-copy-deletes/expect_table_structure create mode 100644 localtests/autoinc-copy-simple/create.sql create mode 100644 localtests/autoinc-copy-simple/expect_table_structure diff --git a/go/base/context.go b/go/base/context.go index 3757653..51b43df 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -206,6 +206,7 @@ type MigrationContext struct { OriginalTableColumns *sql.ColumnList OriginalTableVirtualColumns *sql.ColumnList OriginalTableUniqueKeys [](*sql.UniqueKey) + OriginalTableAutoIncrement uint64 GhostTableColumns *sql.ColumnList GhostTableVirtualColumns *sql.ColumnList GhostTableUniqueKeys [](*sql.UniqueKey) diff --git a/go/logic/applier.go b/go/logic/applier.go index 52628ec..67b519e 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -207,6 +207,25 @@ func (this *Applier) AlterGhost() error { return nil } +// AlterGhost applies `alter` statement on ghost table +func (this *Applier) AlterGhostAutoIncrement() error { + query := fmt.Sprintf(`alter /* gh-ost */ table %s.%s AUTO_INCREMENT=%d`, + sql.EscapeName(this.migrationContext.DatabaseName), + sql.EscapeName(this.migrationContext.GetGhostTableName()), + this.migrationContext.OriginalTableAutoIncrement, + ) + this.migrationContext.Log.Infof("Altering ghost table AUTO_INCREMENT value %s.%s", + sql.EscapeName(this.migrationContext.DatabaseName), + sql.EscapeName(this.migrationContext.GetGhostTableName()), + ) + this.migrationContext.Log.Debugf("AUTO_INCREMENT ALTER statement: %s", query) + if _, err := sqlutils.ExecNoPrepare(this.db, query); err != nil { + return err + } + this.migrationContext.Log.Infof("Ghost table AUTO_INCREMENT altered") + return nil +} + // CreateChangelogTable creates the changelog table on the applier host func (this *Applier) CreateChangelogTable() error { if err := this.DropChangelogTable(); err != nil { diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 584c56b..3e3e303 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -111,6 +111,10 @@ func (this *Inspector) InspectOriginalTable() (err error) { if err != nil { return err } + this.migrationContext.OriginalTableAutoIncrement, err = this.getAutoIncrementValue(this.migrationContext.OriginalTableName) + if err != nil { + return err + } return nil } @@ -596,6 +600,24 @@ func (this *Inspector) applyColumnTypes(databaseName, tableName string, columnsL return err } +// getAutoIncrementValue get's the original table's AUTO_INCREMENT value, if exists (0 value if not exists) +func (this *Inspector) getAutoIncrementValue(tableName string) (autoIncrement uint64, err error) { + query := ` + SELECT + AUTO_INCREMENT + FROM INFORMATION_SCHEMA.TABLES + WHERE + TABLES.TABLE_SCHEMA = ? + AND TABLES.TABLE_NAME = ? + AND AUTO_INCREMENT IS NOT NULL + ` + err = sqlutils.QueryRowsMap(this.db, query, func(m sqlutils.RowMap) error { + autoIncrement = m.GetUint64("AUTO_INCREMENT") + return nil + }, this.migrationContext.DatabaseName, tableName) + return autoIncrement, err +} + // getCandidateUniqueKeys investigates a table and returns the list of unique keys // candidate for chunking func (this *Inspector) getCandidateUniqueKeys(tableName string) (uniqueKeys [](*sql.UniqueKey), err error) { diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 2a038a8..dfddccf 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -1100,6 +1100,14 @@ func (this *Migrator) initiateApplier() error { return err } + if this.migrationContext.OriginalTableAutoIncrement > 0 && !this.parser.IsAutoIncrementDefined() { + // Original table has AUTO_INCREMENT value and the -alter statement does not indicate any override, + // so we should copy AUTO_INCREMENT value onto our ghost table. + if err := this.applier.AlterGhostAutoIncrement(); err != nil { + this.migrationContext.Log.Errorf("Unable to ALTER ghost table AUTO_INCREMENT value, see further error details. Bailing out") + return err + } + } this.applier.WriteChangelogState(string(GhostTableMigrated)) go this.applier.InitiateHeartbeat() return nil diff --git a/go/sql/parser.go b/go/sql/parser.go index ebb8b38..d9c0c3f 100644 --- a/go/sql/parser.go +++ b/go/sql/parser.go @@ -16,6 +16,7 @@ var ( 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+`) + autoIncrementRegexp = regexp.MustCompile(`(?i)\bauto_increment[\s]*=[\s]*([0-9]+)`) alterTableExplicitSchemaTableRegexps = []*regexp.Regexp{ // ALTER TABLE `scm`.`tbl` something regexp.MustCompile(`(?i)\balter\s+table\s+` + "`" + `([^` + "`" + `]+)` + "`" + `[.]` + "`" + `([^` + "`" + `]+)` + "`" + `\s+(.*$)`), @@ -35,9 +36,10 @@ var ( ) type AlterTableParser struct { - columnRenameMap map[string]string - droppedColumns map[string]bool - isRenameTable bool + columnRenameMap map[string]string + droppedColumns map[string]bool + isRenameTable bool + isAutoIncrementDefined bool alterStatementOptions string alterTokens []string @@ -122,6 +124,12 @@ func (this *AlterTableParser) parseAlterToken(alterToken string) (err error) { this.isRenameTable = true } } + { + // auto_increment + if autoIncrementRegexp.MatchString(alterToken) { + this.isAutoIncrementDefined = true + } + } return nil } @@ -173,6 +181,11 @@ func (this *AlterTableParser) DroppedColumnsMap() map[string]bool { func (this *AlterTableParser) IsRenameTable() bool { return this.isRenameTable } + +func (this *AlterTableParser) IsAutoIncrementDefined() bool { + return this.isAutoIncrementDefined +} + func (this *AlterTableParser) GetExplicitSchema() string { return this.explicitSchema } diff --git a/go/sql/parser_test.go b/go/sql/parser_test.go index 79faa63..6cdbb39 100644 --- a/go/sql/parser_test.go +++ b/go/sql/parser_test.go @@ -24,6 +24,7 @@ func TestParseAlterStatement(t *testing.T) { test.S(t).ExpectNil(err) test.S(t).ExpectEquals(parser.alterStatementOptions, statement) test.S(t).ExpectFalse(parser.HasNonTrivialRenames()) + test.S(t).ExpectFalse(parser.IsAutoIncrementDefined()) } func TestParseAlterStatementTrivialRename(t *testing.T) { @@ -33,10 +34,31 @@ func TestParseAlterStatementTrivialRename(t *testing.T) { test.S(t).ExpectNil(err) test.S(t).ExpectEquals(parser.alterStatementOptions, statement) test.S(t).ExpectFalse(parser.HasNonTrivialRenames()) + test.S(t).ExpectFalse(parser.IsAutoIncrementDefined()) test.S(t).ExpectEquals(len(parser.columnRenameMap), 1) test.S(t).ExpectEquals(parser.columnRenameMap["ts"], "ts") } +func TestParseAlterStatementWithAutoIncrement(t *testing.T) { + + statements := []string{ + "auto_increment=7", + "auto_increment = 7", + "AUTO_INCREMENT = 71", + "add column t int, change ts ts timestamp, auto_increment=7 engine=innodb", + "add column t int, change ts ts timestamp, auto_increment =7 engine=innodb", + "add column t int, change ts ts timestamp, AUTO_INCREMENT = 7 engine=innodb", + "add column t int, change ts ts timestamp, engine=innodb auto_increment=73425", + } + for _, statement := range statements { + parser := NewAlterTableParser() + err := parser.ParseAlterStatement(statement) + test.S(t).ExpectNil(err) + test.S(t).ExpectEquals(parser.alterStatementOptions, statement) + test.S(t).ExpectTrue(parser.IsAutoIncrementDefined()) + } +} + func TestParseAlterStatementTrivialRenames(t *testing.T) { statement := "add column t int, change ts ts timestamp, CHANGE f `f` float, engine=innodb" parser := NewAlterTableParser() @@ -44,6 +66,7 @@ func TestParseAlterStatementTrivialRenames(t *testing.T) { test.S(t).ExpectNil(err) test.S(t).ExpectEquals(parser.alterStatementOptions, statement) test.S(t).ExpectFalse(parser.HasNonTrivialRenames()) + test.S(t).ExpectFalse(parser.IsAutoIncrementDefined()) test.S(t).ExpectEquals(len(parser.columnRenameMap), 2) test.S(t).ExpectEquals(parser.columnRenameMap["ts"], "ts") test.S(t).ExpectEquals(parser.columnRenameMap["f"], "f") @@ -64,6 +87,7 @@ func TestParseAlterStatementNonTrivial(t *testing.T) { parser := NewAlterTableParser() err := parser.ParseAlterStatement(statement) test.S(t).ExpectNil(err) + test.S(t).ExpectFalse(parser.IsAutoIncrementDefined()) test.S(t).ExpectEquals(parser.alterStatementOptions, statement) renames := parser.GetNonTrivialRenames() test.S(t).ExpectEquals(len(renames), 2) diff --git a/localtests/autoinc-copy-deletes-user-defined/create.sql b/localtests/autoinc-copy-deletes-user-defined/create.sql new file mode 100644 index 0000000..2058b0b --- /dev/null +++ b/localtests/autoinc-copy-deletes-user-defined/create.sql @@ -0,0 +1,17 @@ +drop event if exists gh_ost_test; + +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + primary key(id) +) auto_increment=1; + +insert into gh_ost_test values (NULL, 11); +insert into gh_ost_test values (NULL, 13); +insert into gh_ost_test values (NULL, 17); +insert into gh_ost_test values (NULL, 23); +insert into gh_ost_test values (NULL, 29); +insert into gh_ost_test values (NULL, 31); +insert into gh_ost_test values (NULL, 37); +delete from gh_ost_test where id>=5; diff --git a/localtests/autoinc-copy-deletes-user-defined/expect_table_structure b/localtests/autoinc-copy-deletes-user-defined/expect_table_structure new file mode 100644 index 0000000..5e180af --- /dev/null +++ b/localtests/autoinc-copy-deletes-user-defined/expect_table_structure @@ -0,0 +1 @@ +AUTO_INCREMENT=7 diff --git a/localtests/autoinc-copy-deletes-user-defined/extra_args b/localtests/autoinc-copy-deletes-user-defined/extra_args new file mode 100644 index 0000000..cce91e1 --- /dev/null +++ b/localtests/autoinc-copy-deletes-user-defined/extra_args @@ -0,0 +1 @@ +--alter='AUTO_INCREMENT=7' diff --git a/localtests/autoinc-copy-deletes/create.sql b/localtests/autoinc-copy-deletes/create.sql new file mode 100644 index 0000000..2058b0b --- /dev/null +++ b/localtests/autoinc-copy-deletes/create.sql @@ -0,0 +1,17 @@ +drop event if exists gh_ost_test; + +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + primary key(id) +) auto_increment=1; + +insert into gh_ost_test values (NULL, 11); +insert into gh_ost_test values (NULL, 13); +insert into gh_ost_test values (NULL, 17); +insert into gh_ost_test values (NULL, 23); +insert into gh_ost_test values (NULL, 29); +insert into gh_ost_test values (NULL, 31); +insert into gh_ost_test values (NULL, 37); +delete from gh_ost_test where id>=5; diff --git a/localtests/autoinc-copy-deletes/expect_table_structure b/localtests/autoinc-copy-deletes/expect_table_structure new file mode 100644 index 0000000..5a755ff --- /dev/null +++ b/localtests/autoinc-copy-deletes/expect_table_structure @@ -0,0 +1 @@ +AUTO_INCREMENT=8 diff --git a/localtests/autoinc-copy-simple/create.sql b/localtests/autoinc-copy-simple/create.sql new file mode 100644 index 0000000..677f08e --- /dev/null +++ b/localtests/autoinc-copy-simple/create.sql @@ -0,0 +1,13 @@ +drop event if exists gh_ost_test; + +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + primary key(id) +) auto_increment=1; + +insert into gh_ost_test values (NULL, 11); +insert into gh_ost_test values (NULL, 13); +insert into gh_ost_test values (NULL, 17); +insert into gh_ost_test values (NULL, 23); diff --git a/localtests/autoinc-copy-simple/expect_table_structure b/localtests/autoinc-copy-simple/expect_table_structure new file mode 100644 index 0000000..3ed5902 --- /dev/null +++ b/localtests/autoinc-copy-simple/expect_table_structure @@ -0,0 +1 @@ +AUTO_INCREMENT=5 diff --git a/localtests/test.sh b/localtests/test.sh index d4b3f17..eb3d640 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -12,6 +12,7 @@ test_logfile=/tmp/gh-ost-test.log default_ghost_binary=/tmp/gh-ost-test ghost_binary="" exec_command_file=/tmp/gh-ost-test.bash +ghost_structure_output_file=/tmp/gh-ost-test.ghost.structure.sql orig_content_output_file=/tmp/gh-ost-test.orig.content.csv ghost_content_output_file=/tmp/gh-ost-test.ghost.content.csv throttle_flag_file=/tmp/gh-ost-test.ghost.throttle.flag @@ -204,6 +205,18 @@ test_single() { return 1 fi + gh-ost-test-mysql-replica --default-character-set=utf8mb4 test -e "show create table _gh_ost_test_gho\G" -ss > $ghost_structure_output_file + + if [ -f $tests_path/$test_name/expect_table_structure ] ; then + expected_table_structure="$(cat $tests_path/$test_name/expect_table_structure)" + if ! grep -q "$expected_table_structure" $ghost_structure_output_file ; then + echo + echo "ERROR $test_name: table structure was expected to include ${expected_table_structure} but did not. cat $ghost_structure_output_file:" + cat $ghost_structure_output_file + return 1 + fi + fi + echo_dot gh-ost-test-mysql-replica --default-character-set=utf8mb4 test -e "select ${orig_columns} from gh_ost_test ${order_by}" -ss > $orig_content_output_file gh-ost-test-mysql-replica --default-character-set=utf8mb4 test -e "select ${ghost_columns} from _gh_ost_test_gho ${order_by}" -ss > $ghost_content_output_file diff --git a/vendor/github.com/outbrain/golib/sqlutils/sqlutils.go b/vendor/github.com/outbrain/golib/sqlutils/sqlutils.go index 8d98690..56761c6 100644 --- a/vendor/github.com/outbrain/golib/sqlutils/sqlutils.go +++ b/vendor/github.com/outbrain/golib/sqlutils/sqlutils.go @@ -117,6 +117,19 @@ func (this *RowMap) GetUintD(key string, def uint) uint { return uint(res) } +func (this *RowMap) GetUint64(key string) uint64 { + res, _ := strconv.ParseUint(this.GetString(key), 10, 0) + return res +} + +func (this *RowMap) GetUint64D(key string, def uint64) uint64 { + res, err := strconv.ParseUint(this.GetString(key), 10, 0) + if err != nil { + return def + } + return uint64(res) +} + func (this *RowMap) GetBool(key string) bool { return this.GetInt(key) != 0 }