From ae22d84ef0d00c3cca010633cf9949f0fdc2dba8 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 5 Aug 2020 12:23:41 +0300 Subject: [PATCH 01/25] v1.1.0 --- RELEASE_VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_VERSION b/RELEASE_VERSION index f1339850d..9084fa2f7 100644 --- a/RELEASE_VERSION +++ b/RELEASE_VERSION @@ -1 +1 @@ -1.0.49 +1.1.0 From 294d43b4f6455bc3dcaf0c88d025778d1e958ca0 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 09:29:38 +0200 Subject: [PATCH 02/25] 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> --- localtests/test.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/localtests/test.sh b/localtests/test.sh index d4b3f1723..05762c288 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" $test_logfile ; then + # return 0 + # fi + # echo + # echo "ERROR $test_name execution was expected to exit with error message '${expected_error_message}' but did not. cat $test_logfile" + # return 1 + # 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 From 26f76027b2806ad2885c465c0f02bcd1c69bb3f8 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 09:48:08 +0200 Subject: [PATCH 03/25] greping for 'expect_table_structure' content --- localtests/test.sh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/localtests/test.sh b/localtests/test.sh index 05762c288..eb3d6403b 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -207,15 +207,15 @@ test_single() { 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" $test_logfile ; then - # return 0 - # fi - # echo - # echo "ERROR $test_name execution was expected to exit with error message '${expected_error_message}' but did not. cat $test_logfile" - # return 1 - # fi + 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 From 75009db84911867a3fc37b3d2efcf442064e5753 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 09:50:56 +0200 Subject: [PATCH 04/25] Adding simple test for 'expect_table_structure' scenario --- localtests/autoinc-copy-simple/create.sql | 11 +++++++++++ localtests/autoinc-copy-simple/expect_table_structure | 1 + 2 files changed, 12 insertions(+) create mode 100644 localtests/autoinc-copy-simple/create.sql create mode 100644 localtests/autoinc-copy-simple/expect_table_structure diff --git a/localtests/autoinc-copy-simple/create.sql b/localtests/autoinc-copy-simple/create.sql new file mode 100644 index 000000000..8aa19bba3 --- /dev/null +++ b/localtests/autoinc-copy-simple/create.sql @@ -0,0 +1,11 @@ +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 000000000..3ed59021b --- /dev/null +++ b/localtests/autoinc-copy-simple/expect_table_structure @@ -0,0 +1 @@ +AUTO_INCREMENT=5 From eeab264eb2436fc0741a7f0626b1fd8a1b80061d Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 09:53:37 +0200 Subject: [PATCH 05/25] adding tests for AUTO_INCREMENT value after row deletes. Should initially fail --- localtests/autoinc-copy-deletes/create.sql | 15 +++++++++++++++ .../autoinc-copy-deletes/expect_table_structure | 1 + 2 files changed, 16 insertions(+) create mode 100644 localtests/autoinc-copy-deletes/create.sql create mode 100644 localtests/autoinc-copy-deletes/expect_table_structure diff --git a/localtests/autoinc-copy-deletes/create.sql b/localtests/autoinc-copy-deletes/create.sql new file mode 100644 index 000000000..dde7f0978 --- /dev/null +++ b/localtests/autoinc-copy-deletes/create.sql @@ -0,0 +1,15 @@ +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 000000000..5a755ffb8 --- /dev/null +++ b/localtests/autoinc-copy-deletes/expect_table_structure @@ -0,0 +1 @@ +AUTO_INCREMENT=8 From 2d0281f29b267739b337c28c4738736d9759054d Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 10:06:12 +0200 Subject: [PATCH 06/25] clear event beforehand --- localtests/autoinc-copy-deletes/create.sql | 2 ++ localtests/autoinc-copy-simple/create.sql | 2 ++ 2 files changed, 4 insertions(+) diff --git a/localtests/autoinc-copy-deletes/create.sql b/localtests/autoinc-copy-deletes/create.sql index dde7f0978..0d668128d 100644 --- a/localtests/autoinc-copy-deletes/create.sql +++ b/localtests/autoinc-copy-deletes/create.sql @@ -1,3 +1,5 @@ +drop event if exists gh_ost_test; + drop table if exists gh_ost_test; create table gh_ost_test ( id int auto_increment, diff --git a/localtests/autoinc-copy-simple/create.sql b/localtests/autoinc-copy-simple/create.sql index 8aa19bba3..677f08e4c 100644 --- a/localtests/autoinc-copy-simple/create.sql +++ b/localtests/autoinc-copy-simple/create.sql @@ -1,3 +1,5 @@ +drop event if exists gh_ost_test; + drop table if exists gh_ost_test; create table gh_ost_test ( id int auto_increment, From af20211629c9a47631749ebb5359ec1c08c3d0a1 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 11:01:13 +0200 Subject: [PATCH 07/25] 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 --- 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 ++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 3 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index cee66efe7..5419d5742 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -203,6 +203,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 089978c25..cc0aa01a3 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -204,6 +204,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 bc1083061..23f238ae6 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -109,6 +109,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 } @@ -589,6 +593,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 70af08db9..10df55581 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -1068,6 +1068,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 ebb8b3883..d9c0c3f19 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 79faa630e..6cdbb39cc 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) From 31069ae4f2334a66760fb3bb430319598d6de253 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 11:06:37 +0200 Subject: [PATCH 08/25] support GetUint64 Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- .../github.com/outbrain/golib/sqlutils/sqlutils.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/vendor/github.com/outbrain/golib/sqlutils/sqlutils.go b/vendor/github.com/outbrain/golib/sqlutils/sqlutils.go index 8d9869078..56761c655 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 } From 3d4dfaafd991eaea1bc7b47bfaf23e6d47ba3700 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 11:06:53 +0200 Subject: [PATCH 09/25] minor update to test Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- localtests/autoinc-copy-deletes/create.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/localtests/autoinc-copy-deletes/create.sql b/localtests/autoinc-copy-deletes/create.sql index 0d668128d..2058b0b84 100644 --- a/localtests/autoinc-copy-deletes/create.sql +++ b/localtests/autoinc-copy-deletes/create.sql @@ -14,4 +14,4 @@ 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; +delete from gh_ost_test where id>=5; From 63219ab3e3629abcfe64630b37d31382599dbaef Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 31 Dec 2020 11:48:46 +0200 Subject: [PATCH 10/25] adding test for user defined AUTO_INCREMENT statement --- go/logic/inspect.go | 6 +++--- .../create.sql | 17 +++++++++++++++++ .../expect_table_structure | 1 + .../extra_args | 1 + 4 files changed, 22 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 diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 23f238ae6..d2633de3a 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -596,10 +596,10 @@ func (this *Inspector) applyColumnTypes(databaseName, tableName string, columnsL // 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 + SELECT AUTO_INCREMENT - FROM INFORMATION_SCHEMA.TABLES - WHERE + FROM INFORMATION_SCHEMA.TABLES + WHERE TABLES.TABLE_SCHEMA = ? AND TABLES.TABLE_NAME = ? AND AUTO_INCREMENT IS NOT NULL 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 000000000..2058b0b84 --- /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 000000000..5e180af3b --- /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 000000000..cce91e1c7 --- /dev/null +++ b/localtests/autoinc-copy-deletes-user-defined/extra_args @@ -0,0 +1 @@ +--alter='AUTO_INCREMENT=7' From 7202076c77c49d175e4283e69c4bad8b08e711ff Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 19 Jan 2021 13:27:00 +0200 Subject: [PATCH 11/25] Generated column as part of UNIQUE (or PRIMARY) KEY Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- .../generated-columns57-unique/create.sql | 30 +++++++++++++++++++ .../ignore_versions | 1 + 2 files changed, 31 insertions(+) create mode 100644 localtests/generated-columns57-unique/create.sql create mode 100644 localtests/generated-columns57-unique/ignore_versions diff --git a/localtests/generated-columns57-unique/create.sql b/localtests/generated-columns57-unique/create.sql new file mode 100644 index 000000000..c75b5ff87 --- /dev/null +++ b/localtests/generated-columns57-unique/create.sql @@ -0,0 +1,30 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + `idb` varchar(36) CHARACTER SET utf8mb4 GENERATED ALWAYS AS (json_unquote(json_extract(`jsonobj`,_utf8mb4'$._id'))) STORED NOT NULL, + `jsonobj` json NOT NULL, + PRIMARY KEY (`id`,`idb`) +) auto_increment=1; + +insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":2}''); +insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":3}''); + +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 (id, jsonobj) values (null, ''{"_id":5}''); + insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":7}''); + insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":11}''); + insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":13}''); + insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":17}''); + insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":19}''); + insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":23}''); + insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":27}''); +end ;; diff --git a/localtests/generated-columns57-unique/ignore_versions b/localtests/generated-columns57-unique/ignore_versions new file mode 100644 index 000000000..b6de5f8d9 --- /dev/null +++ b/localtests/generated-columns57-unique/ignore_versions @@ -0,0 +1 @@ +(5.5|5.6) From b7b3bfbd34a405813549ffd1ce96032520d8cf73 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 19 Jan 2021 13:42:45 +0200 Subject: [PATCH 12/25] skip analysis of generated column data type in unique key Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/logic/inspect.go | 4 ++++ .../generated-columns57-unique/create.sql | 20 +++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/go/logic/inspect.go b/go/logic/inspect.go index fc7001770..f36df0f68 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -188,6 +188,10 @@ func (this *Inspector) inspectOriginalAndGhostTables() (err error) { } for _, column := range this.migrationContext.UniqueKey.Columns.Columns() { + if this.migrationContext.GhostTableVirtualColumns.GetColumn(column.Name) != nil { + // this is a virtual column + continue + } if this.migrationContext.MappedSharedColumns.HasTimezoneConversion(column.Name) { return fmt.Errorf("No support at this time for converting a column from DATETIME to TIMESTAMP that is also part of the chosen unique key. Column: %s, key: %s", column.Name, this.migrationContext.UniqueKey.Name) } diff --git a/localtests/generated-columns57-unique/create.sql b/localtests/generated-columns57-unique/create.sql index c75b5ff87..7a63dd984 100644 --- a/localtests/generated-columns57-unique/create.sql +++ b/localtests/generated-columns57-unique/create.sql @@ -6,8 +6,8 @@ create table gh_ost_test ( PRIMARY KEY (`id`,`idb`) ) auto_increment=1; -insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":2}''); -insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":3}''); +insert into gh_ost_test (id, jsonobj) values (null, '{"_id":2}'); +insert into gh_ost_test (id, jsonobj) values (null, '{"_id":3}'); drop event if exists gh_ost_test; delimiter ;; @@ -19,12 +19,12 @@ create event gh_ost_test enable do begin - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":5}''); - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":7}''); - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":11}''); - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":13}''); - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":17}''); - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":19}''); - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":23}''); - insert into gh_ost_test (id, jsonobj) values (null, ''{"_id":27}''); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":5}'); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":7}'); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":11}'); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":13}'); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":17}'); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":19}'); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":23}'); + insert into gh_ost_test (id, jsonobj) values (null, '{"_id":27}'); end ;; From 710c9ddda575c74b668f211241ea5eb2ec11bcfc Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 18 Feb 2021 10:44:47 +0200 Subject: [PATCH 13/25] All MySQL DBs limited to max 3 concurrent/idle connections Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/logic/throttler.go | 7 +++++-- go/mysql/utils.go | 30 ++++++++++++++++-------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/go/logic/throttler.go b/go/logic/throttler.go index d234ea663..abe8669e1 100644 --- a/go/logic/throttler.go +++ b/go/logic/throttler.go @@ -188,9 +188,12 @@ func (this *Throttler) collectControlReplicasLag() { dbUri := connectionConfig.GetDBUri("information_schema") var heartbeatValue string - if db, _, err := mysql.GetDB(this.migrationContext.Uuid, dbUri); err != nil { + db, _, err := mysql.GetDB(this.migrationContext.Uuid, dbUri) + if err != nil { return lag, err - } else if err = db.QueryRow(replicationLagQuery).Scan(&heartbeatValue); err != nil { + } + + if err := db.QueryRow(replicationLagQuery).Scan(&heartbeatValue); err != nil { return lag, err } diff --git a/go/mysql/utils.go b/go/mysql/utils.go index 17bb5fc32..43a228e18 100644 --- a/go/mysql/utils.go +++ b/go/mysql/utils.go @@ -18,8 +18,11 @@ import ( "github.com/outbrain/golib/sqlutils" ) -const MaxTableNameLength = 64 -const MaxReplicationPasswordLength = 32 +const ( + MaxTableNameLength = 64 + MaxReplicationPasswordLength = 32 + MaxDBPoolConnections = 3 +) type ReplicationLagResult struct { Key InstanceKey @@ -39,23 +42,22 @@ func (this *ReplicationLagResult) HasLag() bool { var knownDBs map[string]*gosql.DB = make(map[string]*gosql.DB) var knownDBsMutex = &sync.Mutex{} -func GetDB(migrationUuid string, mysql_uri string) (*gosql.DB, bool, error) { +func GetDB(migrationUuid string, mysql_uri string) (db *gosql.DB, exists bool, err error) { cacheKey := migrationUuid + ":" + mysql_uri knownDBsMutex.Lock() - defer func() { - knownDBsMutex.Unlock() - }() - - var exists bool - if _, exists = knownDBs[cacheKey]; !exists { - if db, err := gosql.Open("mysql", mysql_uri); err == nil { - knownDBs[cacheKey] = db - } else { - return db, exists, err + defer knownDBsMutex.Unlock() + + if db, exists = knownDBs[cacheKey]; !exists { + db, err = gosql.Open("mysql", mysql_uri) + if err != nil { + return nil, false, err } + db.SetMaxOpenConns(MaxDBPoolConnections) + db.SetMaxIdleConns(MaxDBPoolConnections) + knownDBs[cacheKey] = db } - return knownDBs[cacheKey], exists, nil + return db, exists, nil } // GetReplicationLagFromSlaveStatus returns replication lag for a given db; via SHOW SLAVE STATUS From 54000ab5169921f2cfd5541e06e89f7f2b65797c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 7 Mar 2021 10:47:40 +0200 Subject: [PATCH 14/25] hooks: reporting GH_OST_ETA_SECONDS. ETA stored as part of migration context Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/base/context.go | 9 +++++++++ go/logic/hooks.go | 1 + go/logic/migrator.go | 19 ++++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 51b43dfc8..97d0d60b9 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -182,6 +182,7 @@ type MigrationContext struct { lastHeartbeatOnChangelogMutex *sync.Mutex CurrentLag int64 currentProgress uint64 + etaNanoseonds int64 ThrottleHTTPStatusCode int64 controlReplicasLagResult mysql.ReplicationLagResult TotalRowsCopied int64 @@ -474,6 +475,14 @@ func (this *MigrationContext) SetProgressPct(progressPct float64) { atomic.StoreUint64(&this.currentProgress, math.Float64bits(progressPct)) } +func (this *MigrationContext) GetETADuration() time.Duration { + return time.Duration(atomic.LoadInt64(&this.etaNanoseonds)) +} + +func (this *MigrationContext) SetETADuration(etaDuration time.Duration) { + atomic.StoreInt64(&this.etaNanoseonds, etaDuration.Nanoseconds()) +} + // math.Float64bits([f=0..100]) // GetTotalRowsCopied returns the accurate number of rows being copied (affected) diff --git a/go/logic/hooks.go b/go/logic/hooks.go index 71f070ce3..ea7822e71 100644 --- a/go/logic/hooks.go +++ b/go/logic/hooks.go @@ -66,6 +66,7 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [ env = append(env, fmt.Sprintf("GH_OST_INSPECTED_LAG=%f", this.migrationContext.GetCurrentLagDuration().Seconds())) env = append(env, fmt.Sprintf("GH_OST_HEARTBEAT_LAG=%f", this.migrationContext.TimeSinceLastHeartbeatOnChangelog().Seconds())) env = append(env, fmt.Sprintf("GH_OST_PROGRESS=%f", this.migrationContext.GetProgressPct())) + env = append(env, fmt.Sprintf("GH_OST_ETA_SECONDS=%f", this.migrationContext.GetETADuration().Seconds())) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT=%s", this.migrationContext.HooksHintMessage)) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_OWNER=%s", this.migrationContext.HooksHintOwner)) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_TOKEN=%s", this.migrationContext.HooksHintToken)) diff --git a/go/logic/migrator.go b/go/logic/migrator.go index dfddccffc..e34b2a707 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -939,20 +939,29 @@ func (this *Migrator) printStatus(rule PrintStatusRule, writers ...io.Writer) { } var etaSeconds float64 = math.MaxFloat64 - eta := "N/A" + var etaDuration = time.Duration(math.MaxInt64) if progressPct >= 100.0 { - eta = "due" + etaDuration = 0 } else if progressPct >= 0.1 { elapsedRowCopySeconds := this.migrationContext.ElapsedRowCopyTime().Seconds() totalExpectedSeconds := elapsedRowCopySeconds * float64(rowsEstimate) / float64(totalRowsCopied) etaSeconds = totalExpectedSeconds - elapsedRowCopySeconds if etaSeconds >= 0 { - etaDuration := time.Duration(etaSeconds) * time.Second - eta = base.PrettifyDurationOutput(etaDuration) + etaDuration = time.Duration(etaSeconds) * time.Second } else { - eta = "due" + etaDuration = 0 } } + this.migrationContext.SetETADuration(etaDuration) + var eta string + switch etaDuration { + case 0: + eta = "due" + case time.Duration(math.MaxInt64): + eta = "N/A" + default: + eta = base.PrettifyDurationOutput(etaDuration) + } state := "migrating" if atomic.LoadInt64(&this.migrationContext.CountingRowsFlag) > 0 && !this.migrationContext.ConcurrentCountTableRows { From 51719a2b769f72f5eee3c869fa1b7eb602d376fd Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 7 Mar 2021 11:11:50 +0200 Subject: [PATCH 15/25] GH_OST_ETA_NANOSECONDS Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/logic/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/logic/hooks.go b/go/logic/hooks.go index ea7822e71..481c3dfe1 100644 --- a/go/logic/hooks.go +++ b/go/logic/hooks.go @@ -66,7 +66,7 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [ env = append(env, fmt.Sprintf("GH_OST_INSPECTED_LAG=%f", this.migrationContext.GetCurrentLagDuration().Seconds())) env = append(env, fmt.Sprintf("GH_OST_HEARTBEAT_LAG=%f", this.migrationContext.TimeSinceLastHeartbeatOnChangelog().Seconds())) env = append(env, fmt.Sprintf("GH_OST_PROGRESS=%f", this.migrationContext.GetProgressPct())) - env = append(env, fmt.Sprintf("GH_OST_ETA_SECONDS=%f", this.migrationContext.GetETADuration().Seconds())) + env = append(env, fmt.Sprintf("GH_OST_ETA_NANOSECONDS=%d", this.migrationContext.GetETADuration().Nanoseconds())) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT=%s", this.migrationContext.HooksHintMessage)) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_OWNER=%s", this.migrationContext.HooksHintOwner)) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_TOKEN=%s", this.migrationContext.HooksHintToken)) From 76b9c16a68b43f24bfd658cfee8a71cf7dd74806 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 7 Mar 2021 11:27:50 +0200 Subject: [PATCH 16/25] N/A denoted by negative value Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/base/context.go | 5 +++++ go/logic/hooks.go | 2 +- go/logic/migrator.go | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 97d0d60b9..414ead459 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -483,6 +483,11 @@ func (this *MigrationContext) SetETADuration(etaDuration time.Duration) { atomic.StoreInt64(&this.etaNanoseonds, etaDuration.Nanoseconds()) } +func (this *MigrationContext) GetETASeconds() int64 { + nano := atomic.LoadInt64(&this.etaNanoseonds) + return nano / int64(time.Second) +} + // math.Float64bits([f=0..100]) // GetTotalRowsCopied returns the accurate number of rows being copied (affected) diff --git a/go/logic/hooks.go b/go/logic/hooks.go index 481c3dfe1..2275ede51 100644 --- a/go/logic/hooks.go +++ b/go/logic/hooks.go @@ -66,7 +66,7 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [ env = append(env, fmt.Sprintf("GH_OST_INSPECTED_LAG=%f", this.migrationContext.GetCurrentLagDuration().Seconds())) env = append(env, fmt.Sprintf("GH_OST_HEARTBEAT_LAG=%f", this.migrationContext.TimeSinceLastHeartbeatOnChangelog().Seconds())) env = append(env, fmt.Sprintf("GH_OST_PROGRESS=%f", this.migrationContext.GetProgressPct())) - env = append(env, fmt.Sprintf("GH_OST_ETA_NANOSECONDS=%d", this.migrationContext.GetETADuration().Nanoseconds())) + env = append(env, fmt.Sprintf("GH_OST_ETA_SECONDS=%d", this.migrationContext.GetETASeconds())) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT=%s", this.migrationContext.HooksHintMessage)) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_OWNER=%s", this.migrationContext.HooksHintOwner)) env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_TOKEN=%s", this.migrationContext.HooksHintToken)) diff --git a/go/logic/migrator.go b/go/logic/migrator.go index e34b2a707..9ef90e152 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -939,7 +939,7 @@ func (this *Migrator) printStatus(rule PrintStatusRule, writers ...io.Writer) { } var etaSeconds float64 = math.MaxFloat64 - var etaDuration = time.Duration(math.MaxInt64) + var etaDuration = time.Duration(math.MinInt64) if progressPct >= 100.0 { etaDuration = 0 } else if progressPct >= 0.1 { @@ -957,7 +957,7 @@ func (this *Migrator) printStatus(rule PrintStatusRule, writers ...io.Writer) { switch etaDuration { case 0: eta = "due" - case time.Duration(math.MaxInt64): + case time.Duration(math.MinInt64): eta = "N/A" default: eta = base.PrettifyDurationOutput(etaDuration) From b688c58a45dfc34fe4bc59eb84d1c3504bc8c2ac Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 7 Mar 2021 14:16:04 +0200 Subject: [PATCH 17/25] ETAUnknown constant Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/base/context.go | 5 +++++ go/logic/migrator.go | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/go/base/context.go b/go/base/context.go index 414ead459..b9badc43b 100644 --- a/go/base/context.go +++ b/go/base/context.go @@ -52,6 +52,7 @@ const ( const ( HTTPStatusOK = 200 MaxEventsBatchSize = 1000 + ETAUnknown = math.MinInt64 ) var ( @@ -268,6 +269,7 @@ func NewMigrationContext() *MigrationContext { MaxLagMillisecondsThrottleThreshold: 1500, CutOverLockTimeoutSeconds: 3, DMLBatchSize: 10, + etaNanoseonds: ETAUnknown, maxLoad: NewLoadMap(), criticalLoad: NewLoadMap(), throttleMutex: &sync.Mutex{}, @@ -485,6 +487,9 @@ func (this *MigrationContext) SetETADuration(etaDuration time.Duration) { func (this *MigrationContext) GetETASeconds() int64 { nano := atomic.LoadInt64(&this.etaNanoseonds) + if nano < 0 { + return ETAUnknown + } return nano / int64(time.Second) } diff --git a/go/logic/migrator.go b/go/logic/migrator.go index 9ef90e152..c12c21fc3 100644 --- a/go/logic/migrator.go +++ b/go/logic/migrator.go @@ -939,7 +939,7 @@ func (this *Migrator) printStatus(rule PrintStatusRule, writers ...io.Writer) { } var etaSeconds float64 = math.MaxFloat64 - var etaDuration = time.Duration(math.MinInt64) + var etaDuration = time.Duration(base.ETAUnknown) if progressPct >= 100.0 { etaDuration = 0 } else if progressPct >= 0.1 { @@ -957,7 +957,7 @@ func (this *Migrator) printStatus(rule PrintStatusRule, writers ...io.Writer) { switch etaDuration { case 0: eta = "due" - case time.Duration(math.MinInt64): + case time.Duration(base.ETAUnknown): eta = "N/A" default: eta = base.PrettifyDurationOutput(etaDuration) From c1bfe94b0fa744ee96c973f77211fa32ce60c6d7 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 May 2021 18:12:35 +0300 Subject: [PATCH 18/25] Convering enum to varchar Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- localtests/enum-to-varchar/create.sql | 29 +++++++++++++++++++++++++++ localtests/enum-to-varchar/extra_args | 1 + 2 files changed, 30 insertions(+) create mode 100644 localtests/enum-to-varchar/create.sql create mode 100644 localtests/enum-to-varchar/extra_args diff --git a/localtests/enum-to-varchar/create.sql b/localtests/enum-to-varchar/create.sql new file mode 100644 index 000000000..8b25e398b --- /dev/null +++ b/localtests/enum-to-varchar/create.sql @@ -0,0 +1,29 @@ +drop table if exists gh_ost_test; +create table gh_ost_test ( + id int auto_increment, + i int not null, + e enum('red', 'green', 'blue', 'orange') null default null collate 'utf8_bin', + primary key(id) +) auto_increment=1; + +insert into gh_ost_test values (null, 7, 'red'); + +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, 'red'); + insert into gh_ost_test values (null, 13, 'green'); + insert into gh_ost_test values (null, 17, 'blue'); + set @last_insert_id := last_insert_id(); + update gh_ost_test set e='orange' where id = @last_insert_id; + insert into gh_ost_test values (null, 23, null); + set @last_insert_id := last_insert_id(); + update gh_ost_test set i=i+1, e=null where id = @last_insert_id; +end ;; diff --git a/localtests/enum-to-varchar/extra_args b/localtests/enum-to-varchar/extra_args new file mode 100644 index 000000000..68524e49b --- /dev/null +++ b/localtests/enum-to-varchar/extra_args @@ -0,0 +1 @@ +--alter="change e e varchar(32) not null default ''" From 9bb2daaf158a67054addd1e69f609754ad004390 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 May 2021 18:35:52 +0300 Subject: [PATCH 19/25] test: not null Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- localtests/enum-to-varchar/create.sql | 3 --- 1 file changed, 3 deletions(-) diff --git a/localtests/enum-to-varchar/create.sql b/localtests/enum-to-varchar/create.sql index 8b25e398b..0dbab17ed 100644 --- a/localtests/enum-to-varchar/create.sql +++ b/localtests/enum-to-varchar/create.sql @@ -23,7 +23,4 @@ begin insert into gh_ost_test values (null, 17, 'blue'); set @last_insert_id := last_insert_id(); update gh_ost_test set e='orange' where id = @last_insert_id; - insert into gh_ost_test values (null, 23, null); - set @last_insert_id := last_insert_id(); - update gh_ost_test set i=i+1, e=null where id = @last_insert_id; end ;; From 939b898ea9cec4964d0d03a27eeed977982cd05c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 May 2021 19:28:19 +0300 Subject: [PATCH 20/25] first attempt at setting enum-to-string right Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/logic/inspect.go | 3 +++ go/sql/builder.go | 2 ++ go/sql/types.go | 19 ++++++++++++++----- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/go/logic/inspect.go b/go/logic/inspect.go index e61f97bdd..4ad0131ca 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -187,6 +187,9 @@ func (this *Inspector) inspectOriginalAndGhostTables() (err error) { if column.Name == mappedColumn.Name && column.Type == sql.DateTimeColumnType && mappedColumn.Type == sql.TimestampColumnType { this.migrationContext.MappedSharedColumns.SetConvertDatetimeToTimestamp(column.Name, this.migrationContext.ApplierTimeZone) } + if column.Name == mappedColumn.Name && column.Type == sql.EnumColumnType && mappedColumn.Charset != "" { + this.migrationContext.MappedSharedColumns.SetEnumToTextConversion(column.Name) + } } for _, column := range this.migrationContext.UniqueKey.Columns.Columns() { diff --git a/go/sql/builder.go b/go/sql/builder.go index 4b019bcf7..4b970e902 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -108,6 +108,8 @@ func BuildSetPreparedClause(columns *ColumnList) (result string, err error) { var setToken string if column.timezoneConversion != nil { setToken = fmt.Sprintf("%s=convert_tz(?, '%s', '%s')", EscapeName(column.Name), column.timezoneConversion.ToTimezone, "+00:00") + } else if column.enumToTextConversion { + setToken = fmt.Sprintf("%s=concat('', ?)", EscapeName(column.Name)) } else if column.Type == JSONColumnType { setToken = fmt.Sprintf("%s=convert(? using utf8mb4)", EscapeName(column.Name)) } else { diff --git a/go/sql/types.go b/go/sql/types.go index ef8381952..30db6464d 100644 --- a/go/sql/types.go +++ b/go/sql/types.go @@ -31,11 +31,12 @@ type TimezoneConversion struct { } type Column struct { - Name string - IsUnsigned bool - Charset string - Type ColumnType - timezoneConversion *TimezoneConversion + Name string + IsUnsigned bool + Charset string + Type ColumnType + timezoneConversion *TimezoneConversion + enumToTextConversion bool } func (this *Column) convertArg(arg interface{}) interface{} { @@ -179,6 +180,14 @@ func (this *ColumnList) HasTimezoneConversion(columnName string) bool { return this.GetColumn(columnName).timezoneConversion != nil } +func (this *ColumnList) SetEnumToTextConversion(columnName string) { + this.GetColumn(columnName).enumToTextConversion = true +} + +func (this *ColumnList) IsEnumToTextConversion(columnName string) bool { + return this.GetColumn(columnName).enumToTextConversion +} + func (this *ColumnList) String() string { return strings.Join(this.Names(), ",") } From 95ee9e2144ff97b75638822590d24497205f0da7 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 May 2021 19:37:25 +0300 Subject: [PATCH 21/25] fix insert query Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/sql/builder.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/sql/builder.go b/go/sql/builder.go index 4b970e902..4618ef179 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -38,6 +38,8 @@ func buildColumnsPreparedValues(columns *ColumnList) []string { var token string if column.timezoneConversion != nil { token = fmt.Sprintf("convert_tz(?, '%s', '%s')", column.timezoneConversion.ToTimezone, "+00:00") + } else if column.enumToTextConversion { + token = "concat('', ?)" } else if column.Type == JSONColumnType { token = "convert(? using utf8mb4)" } else { From e80ddb42c9c488eac0c950711b0f6e4cb4cd2215 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 May 2021 20:44:39 +0300 Subject: [PATCH 22/25] store enum values, use when populating Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/logic/inspect.go | 1 + go/sql/builder.go | 4 ++-- go/sql/parser.go | 8 ++++++++ go/sql/parser_test.go | 18 ++++++++++++++++++ go/sql/types.go | 1 + 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 4ad0131ca..7b8002d4d 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -592,6 +592,7 @@ func (this *Inspector) applyColumnTypes(databaseName, tableName string, columnsL } if strings.HasPrefix(columnType, "enum") { column.Type = sql.EnumColumnType + column.EnumValues = sql.ParseEnumValues(m.GetString("COLUMN_TYPE")) } if charset := m.GetString("CHARACTER_SET_NAME"); charset != "" { column.Charset = charset diff --git a/go/sql/builder.go b/go/sql/builder.go index 4618ef179..878850c54 100644 --- a/go/sql/builder.go +++ b/go/sql/builder.go @@ -39,7 +39,7 @@ func buildColumnsPreparedValues(columns *ColumnList) []string { if column.timezoneConversion != nil { token = fmt.Sprintf("convert_tz(?, '%s', '%s')", column.timezoneConversion.ToTimezone, "+00:00") } else if column.enumToTextConversion { - token = "concat('', ?)" + token = fmt.Sprintf("ELT(?, %s)", column.EnumValues) } else if column.Type == JSONColumnType { token = "convert(? using utf8mb4)" } else { @@ -111,7 +111,7 @@ func BuildSetPreparedClause(columns *ColumnList) (result string, err error) { if column.timezoneConversion != nil { setToken = fmt.Sprintf("%s=convert_tz(?, '%s', '%s')", EscapeName(column.Name), column.timezoneConversion.ToTimezone, "+00:00") } else if column.enumToTextConversion { - setToken = fmt.Sprintf("%s=concat('', ?)", EscapeName(column.Name)) + setToken = fmt.Sprintf("%s=ELT(?, %s)", EscapeName(column.Name), column.EnumValues) } else if column.Type == JSONColumnType { setToken = fmt.Sprintf("%s=convert(? using utf8mb4)", EscapeName(column.Name)) } else { diff --git a/go/sql/parser.go b/go/sql/parser.go index d9c0c3f19..eac0bdce3 100644 --- a/go/sql/parser.go +++ b/go/sql/parser.go @@ -33,6 +33,7 @@ var ( // ALTER TABLE tbl something regexp.MustCompile(`(?i)\balter\s+table\s+([\S]+)\s+(.*$)`), } + enumValuesRegexp = regexp.MustCompile("^enum[(](.*)[)]$") ) type AlterTableParser struct { @@ -205,3 +206,10 @@ func (this *AlterTableParser) HasExplicitTable() bool { func (this *AlterTableParser) GetAlterStatementOptions() string { return this.alterStatementOptions } + +func ParseEnumValues(enumColumnType string) string { + if submatch := enumValuesRegexp.FindStringSubmatch(enumColumnType); len(submatch) > 0 { + return submatch[1] + } + return enumColumnType +} diff --git a/go/sql/parser_test.go b/go/sql/parser_test.go index 6cdbb39cc..3157d097e 100644 --- a/go/sql/parser_test.go +++ b/go/sql/parser_test.go @@ -322,3 +322,21 @@ func TestParseAlterStatementExplicitTable(t *testing.T) { test.S(t).ExpectTrue(reflect.DeepEqual(parser.alterTokens, []string{"drop column b", "add index idx(i)"})) } } + +func TestParseEnumValues(t *testing.T) { + { + s := "enum('red','green','blue','orange')" + values := ParseEnumValues(s) + test.S(t).ExpectEquals(values, "'red','green','blue','orange'") + } + { + s := "('red','green','blue','orange')" + values := ParseEnumValues(s) + test.S(t).ExpectEquals(values, "('red','green','blue','orange')") + } + { + s := "zzz" + values := ParseEnumValues(s) + test.S(t).ExpectEquals(values, "zzz") + } +} diff --git a/go/sql/types.go b/go/sql/types.go index 30db6464d..44e972548 100644 --- a/go/sql/types.go +++ b/go/sql/types.go @@ -35,6 +35,7 @@ type Column struct { IsUnsigned bool Charset string Type ColumnType + EnumValues string timezoneConversion *TimezoneConversion enumToTextConversion bool } From 6e5b665c557e2aa4c1456410a0f231022f950966 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 May 2021 20:56:02 +0300 Subject: [PATCH 23/25] apply EnumValues to mapped column Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/logic/inspect.go | 1 + go/sql/types.go | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/go/logic/inspect.go b/go/logic/inspect.go index 7b8002d4d..6c873c85e 100644 --- a/go/logic/inspect.go +++ b/go/logic/inspect.go @@ -189,6 +189,7 @@ func (this *Inspector) inspectOriginalAndGhostTables() (err error) { } if column.Name == mappedColumn.Name && column.Type == sql.EnumColumnType && mappedColumn.Charset != "" { this.migrationContext.MappedSharedColumns.SetEnumToTextConversion(column.Name) + this.migrationContext.MappedSharedColumns.SetEnumValues(column.Name, column.EnumValues) } } diff --git a/go/sql/types.go b/go/sql/types.go index 44e972548..c4be37100 100644 --- a/go/sql/types.go +++ b/go/sql/types.go @@ -189,6 +189,10 @@ func (this *ColumnList) IsEnumToTextConversion(columnName string) bool { return this.GetColumn(columnName).enumToTextConversion } +func (this *ColumnList) SetEnumValues(columnName string, enumValues string) { + this.GetColumn(columnName).EnumValues = enumValues + + func (this *ColumnList) String() string { return strings.Join(this.Names(), ",") } From 82bdf066e9edbeb9e0887ec15121fc4e27e50bc3 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 2 May 2021 20:59:50 +0300 Subject: [PATCH 24/25] fix compilation error Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/sql/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/sql/types.go b/go/sql/types.go index c4be37100..c005316aa 100644 --- a/go/sql/types.go +++ b/go/sql/types.go @@ -191,7 +191,7 @@ func (this *ColumnList) IsEnumToTextConversion(columnName string) bool { func (this *ColumnList) SetEnumValues(columnName string, enumValues string) { this.GetColumn(columnName).EnumValues = enumValues - +} func (this *ColumnList) String() string { return strings.Join(this.Names(), ",") From 48b02833ba832a8916d7c9c71fd3e0c0f2b76de0 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 4 May 2021 18:46:20 +0300 Subject: [PATCH 25/25] gofmt Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/sql/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/sql/types.go b/go/sql/types.go index 76750dbb3..3c4ce5e85 100644 --- a/go/sql/types.go +++ b/go/sql/types.go @@ -42,7 +42,7 @@ type Column struct { enumToTextConversion bool // add Octet length for binary type, fix bytes with suffix "00" get clipped in mysql binlog. // https://github.com/github/gh-ost/issues/909 - BinaryOctetLength uint + BinaryOctetLength uint } func (this *Column) convertArg(arg interface{}, isUniqueKeyColumn bool) interface{} {