From b796a00c940bf8f9b2bdec67af08bcf2c0d452dc Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Wed, 8 Nov 2017 10:08:20 +0200 Subject: [PATCH 1/7] Update to migration's unique key column data loss: fixing --- localtests/update-pk-column/create.sql | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 localtests/update-pk-column/create.sql diff --git a/localtests/update-pk-column/create.sql b/localtests/update-pk-column/create.sql new file mode 100644 index 0000000..348dcdd --- /dev/null +++ b/localtests/update-pk-column/create.sql @@ -0,0 +1,45 @@ +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, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); +insert into gh_ost_test values (null, 13); + + +drop event if exists gh_ost_test; +delimiter ;; +create event gh_ost_test + on schedule every 1 second + starts current_timestamp + interval 3 second + ends current_timestamp + interval 60 second + on completion not preserve + enable + do +begin + update gh_ost_test set id=-2 where id=21; +end ;; From 276311b58a578c143cb1d4fda9e285c028cb94d9 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Wed, 8 Nov 2017 10:13:50 +0200 Subject: [PATCH 2/7] clearer test table values --- localtests/update-pk-column/create.sql | 49 +++++++++++++------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/localtests/update-pk-column/create.sql b/localtests/update-pk-column/create.sql index 348dcdd..3de5098 100644 --- a/localtests/update-pk-column/create.sql +++ b/localtests/update-pk-column/create.sql @@ -5,31 +5,30 @@ create table gh_ost_test ( primary key(id) ) auto_increment=1; -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); -insert into gh_ost_test values (null, 13); - +insert into gh_ost_test values (null, 101); +insert into gh_ost_test values (null, 102); +insert into gh_ost_test values (null, 103); +insert into gh_ost_test values (null, 104); +insert into gh_ost_test values (null, 105); +insert into gh_ost_test values (null, 106); +insert into gh_ost_test values (null, 107); +insert into gh_ost_test values (null, 108); +insert into gh_ost_test values (null, 109); +insert into gh_ost_test values (null, 110); +insert into gh_ost_test values (null, 111); +insert into gh_ost_test values (null, 112); +insert into gh_ost_test values (null, 113); +insert into gh_ost_test values (null, 114); +insert into gh_ost_test values (null, 115); +insert into gh_ost_test values (null, 116); +insert into gh_ost_test values (null, 117); +insert into gh_ost_test values (null, 118); +insert into gh_ost_test values (null, 119); +insert into gh_ost_test values (null, 120); +insert into gh_ost_test values (null, 121); +insert into gh_ost_test values (null, 122); +insert into gh_ost_test values (null, 123); +insert into gh_ost_test values (null, 124); drop event if exists gh_ost_test; delimiter ;; From e740d4b3827e0d5cddeaf469fb25d0713779672c Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Wed, 8 Nov 2017 11:11:14 +0200 Subject: [PATCH 3/7] failing on PK value change --- localtests/fail-update-pk-column/create.sql | 52 +++++++++++++++++++ .../fail-update-pk-column/expect_failure | 1 + 2 files changed, 53 insertions(+) create mode 100644 localtests/fail-update-pk-column/create.sql create mode 100644 localtests/fail-update-pk-column/expect_failure diff --git a/localtests/fail-update-pk-column/create.sql b/localtests/fail-update-pk-column/create.sql new file mode 100644 index 0000000..5cc1d37 --- /dev/null +++ b/localtests/fail-update-pk-column/create.sql @@ -0,0 +1,52 @@ +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, 101); +insert into gh_ost_test values (null, 102); +insert into gh_ost_test values (null, 103); +insert into gh_ost_test values (null, 104); +insert into gh_ost_test values (null, 105); +insert into gh_ost_test values (null, 106); +insert into gh_ost_test values (null, 107); +insert into gh_ost_test values (null, 108); +insert into gh_ost_test values (null, 109); +insert into gh_ost_test values (null, 110); +insert into gh_ost_test values (null, 111); +insert into gh_ost_test values (null, 112); +insert into gh_ost_test values (null, 113); +insert into gh_ost_test values (null, 114); +insert into gh_ost_test values (null, 115); +insert into gh_ost_test values (null, 116); +insert into gh_ost_test values (null, 117); +insert into gh_ost_test values (null, 118); +insert into gh_ost_test values (null, 119); +insert into gh_ost_test values (null, 120); +insert into gh_ost_test values (null, 121); +insert into gh_ost_test values (null, 122); +insert into gh_ost_test values (null, 123); +insert into gh_ost_test values (null, 124); +insert into gh_ost_test values (null, 125); +insert into gh_ost_test values (null, 126); +insert into gh_ost_test values (null, 127); +insert into gh_ost_test values (null, 128); +insert into gh_ost_test values (null, 129); + +drop event if exists gh_ost_test; +delimiter ;; +create event gh_ost_test + on schedule every 1 second + starts current_timestamp + interval 3 second + ends current_timestamp + interval 60 second + on completion not preserve + enable + do +begin + update gh_ost_test set id=-2 where id=21; + update gh_ost_test set id=55 where id=22; + update gh_ost_test set id=23 where id=23; + update gh_ost_test set i=5024 where id=24; +end ;; diff --git a/localtests/fail-update-pk-column/expect_failure b/localtests/fail-update-pk-column/expect_failure new file mode 100644 index 0000000..1b73369 --- /dev/null +++ b/localtests/fail-update-pk-column/expect_failure @@ -0,0 +1 @@ +gh-ost detected an UPDATE to a unique key column this migration is iterating on From 8a59d7e8239dce9f654406c17f9edcfdcc02afa1 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Wed, 8 Nov 2017 11:11:17 +0200 Subject: [PATCH 4/7] failing on PK value change --- go/logic/applier.go | 22 +++++++++++++ localtests/test.sh | 5 +++ localtests/update-pk-column/create.sql | 44 -------------------------- 3 files changed, 27 insertions(+), 44 deletions(-) delete mode 100644 localtests/update-pk-column/create.sql diff --git a/go/logic/applier.go b/go/logic/applier.go index 2607f30..a1175b1 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -899,6 +899,25 @@ func (this *Applier) ShowStatusVariable(variableName string) (result int64, err return result, nil } +func (this *Applier) validateUpdateDoesNotModifyMigrationUniqueKeyColumns(dmlEvent *binlog.BinlogDMLEvent) error { + // log.Debugf("............ UPDATE") + // log.Debugf("............ UPDATE: %+v", this.migrationContext.UniqueKey.Columns.String()) + // log.Debugf("............ UPDATE: %+v", dmlEvent.WhereColumnValues.String()) + // log.Debugf("............ UPDATE: %+v", dmlEvent.NewColumnValues.String()) + for _, column := range this.migrationContext.UniqueKey.Columns.Columns() { + tableOrdinal := this.migrationContext.OriginalTableColumns.Ordinals[column.Name] + whereColumnValue := dmlEvent.WhereColumnValues.AbstractValues()[tableOrdinal] + newColumnValue := dmlEvent.NewColumnValues.AbstractValues()[tableOrdinal] + // log.Debugf("............ UPDATE: old value= %+v", whereColumnValue) + // log.Debugf("............ UPDATE: new value= %+v", newColumnValue) + // log.Debugf("............ UPDATE: equals? %+v", newColumnValue == whereColumnValue) + if newColumnValue != whereColumnValue { + return log.Errorf("gh-ost detected an UPDATE to a unique key column this migration is iterating on. Such update is not supported. Column is %s", column.Name) + } + } + return nil +} + // buildDMLEventQuery creates a query to operate on the ghost table, based on an intercepted binlog // event entry on the original table. func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (query string, args []interface{}, rowsDelta int64, err error) { @@ -915,6 +934,9 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (query } case binlog.UpdateDML: { + if err := this.validateUpdateDoesNotModifyMigrationUniqueKeyColumns(dmlEvent); err != nil { + return query, args, rowsDelta, err + } query, sharedArgs, uniqueKeyArgs, err := sql.BuildDMLUpdateQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.SharedColumns, this.migrationContext.MappedSharedColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues()) args = append(args, sharedArgs...) args = append(args, uniqueKeyArgs...) diff --git a/localtests/test.sh b/localtests/test.sh index b901ce3..477ecbb 100755 --- a/localtests/test.sh +++ b/localtests/test.sh @@ -170,7 +170,12 @@ test_single() { build_binary() { echo "Building" + rm -f $ghost_binary go build -o $ghost_binary go/cmd/gh-ost/main.go + if [ $? -ne 0 ] ; then + echo "Build failure" + exit 1 + fi } test_all() { diff --git a/localtests/update-pk-column/create.sql b/localtests/update-pk-column/create.sql deleted file mode 100644 index 3de5098..0000000 --- a/localtests/update-pk-column/create.sql +++ /dev/null @@ -1,44 +0,0 @@ -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, 101); -insert into gh_ost_test values (null, 102); -insert into gh_ost_test values (null, 103); -insert into gh_ost_test values (null, 104); -insert into gh_ost_test values (null, 105); -insert into gh_ost_test values (null, 106); -insert into gh_ost_test values (null, 107); -insert into gh_ost_test values (null, 108); -insert into gh_ost_test values (null, 109); -insert into gh_ost_test values (null, 110); -insert into gh_ost_test values (null, 111); -insert into gh_ost_test values (null, 112); -insert into gh_ost_test values (null, 113); -insert into gh_ost_test values (null, 114); -insert into gh_ost_test values (null, 115); -insert into gh_ost_test values (null, 116); -insert into gh_ost_test values (null, 117); -insert into gh_ost_test values (null, 118); -insert into gh_ost_test values (null, 119); -insert into gh_ost_test values (null, 120); -insert into gh_ost_test values (null, 121); -insert into gh_ost_test values (null, 122); -insert into gh_ost_test values (null, 123); -insert into gh_ost_test values (null, 124); - -drop event if exists gh_ost_test; -delimiter ;; -create event gh_ost_test - on schedule every 1 second - starts current_timestamp + interval 3 second - ends current_timestamp + interval 60 second - on completion not preserve - enable - do -begin - update gh_ost_test set id=-2 where id=21; -end ;; From 3898d49f6c362a06225aa35fad70fe1e16825e8a Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 20 Nov 2017 08:17:20 +0200 Subject: [PATCH 5/7] supporting update to columns of migration key --- go/logic/applier.go | 152 +++++++++++------- .../fail-update-pk-column/expect_failure | 1 - 2 files changed, 94 insertions(+), 59 deletions(-) delete mode 100644 localtests/fail-update-pk-column/expect_failure diff --git a/go/logic/applier.go b/go/logic/applier.go index a1175b1..a355796 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -24,6 +24,32 @@ const ( atomicCutOverMagicHint = "ghost-cut-over-sentry" ) +type dmlBuildResult struct { + query string + args []interface{} + rowsDelta int64 + err error +} + +func newDmlBuildResult(query string, args []interface{}, rowsDelta int64, err error) *dmlBuildResult { + return &dmlBuildResult{ + query: query, + args: args, + rowsDelta: rowsDelta, + err: err, + } +} + +func newDmlBuildResultError(err error) *dmlBuildResult { + return &dmlBuildResult{ + err: err, + } +} + +func buildResults(args ...(*dmlBuildResult)) [](*dmlBuildResult) { + return args +} + // Applier connects and writes the the applier-server, which is the server where migration // happens. This is typically the master, but could be a replica when `--test-on-replica` or // `--execute-on-replica` are given. @@ -899,7 +925,7 @@ func (this *Applier) ShowStatusVariable(variableName string) (result int64, err return result, nil } -func (this *Applier) validateUpdateDoesNotModifyMigrationUniqueKeyColumns(dmlEvent *binlog.BinlogDMLEvent) error { +func (this *Applier) updateModifiesUniqueKeyColumns(dmlEvent *binlog.BinlogDMLEvent) (modifiedColumn string, isModified bool) { // log.Debugf("............ UPDATE") // log.Debugf("............ UPDATE: %+v", this.migrationContext.UniqueKey.Columns.String()) // log.Debugf("............ UPDATE: %+v", dmlEvent.WhereColumnValues.String()) @@ -912,88 +938,97 @@ func (this *Applier) validateUpdateDoesNotModifyMigrationUniqueKeyColumns(dmlEve // log.Debugf("............ UPDATE: new value= %+v", newColumnValue) // log.Debugf("............ UPDATE: equals? %+v", newColumnValue == whereColumnValue) if newColumnValue != whereColumnValue { - return log.Errorf("gh-ost detected an UPDATE to a unique key column this migration is iterating on. Such update is not supported. Column is %s", column.Name) + return column.Name, true } } - return nil + return "", false } // buildDMLEventQuery creates a query to operate on the ghost table, based on an intercepted binlog // event entry on the original table. -func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (query string, args []interface{}, rowsDelta int64, err error) { +func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (results [](*dmlBuildResult)) { switch dmlEvent.DML { case binlog.DeleteDML: { query, uniqueKeyArgs, err := sql.BuildDMLDeleteQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.WhereColumnValues.AbstractValues()) - return query, uniqueKeyArgs, -1, err + return buildResults(newDmlBuildResult(query, uniqueKeyArgs, -1, err)) } case binlog.InsertDML: { query, sharedArgs, err := sql.BuildDMLInsertQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.SharedColumns, this.migrationContext.MappedSharedColumns, dmlEvent.NewColumnValues.AbstractValues()) - return query, sharedArgs, 1, err + return buildResults(newDmlBuildResult(query, sharedArgs, 1, err)) } case binlog.UpdateDML: { - if err := this.validateUpdateDoesNotModifyMigrationUniqueKeyColumns(dmlEvent); err != nil { - return query, args, rowsDelta, err + if modifiedColumn, isModified := this.updateModifiesUniqueKeyColumns(dmlEvent); isModified { + log.Debugf("---------------- Detected modifiedColumn: %+v. Will turn into DELETE+INSERT", modifiedColumn) + dmlEvent.DML = binlog.DeleteDML + results = append(results, this.buildDMLEventQuery(dmlEvent)...) + dmlEvent.DML = binlog.InsertDML + results = append(results, this.buildDMLEventQuery(dmlEvent)...) + return results + // return buildResults(newDmlBuildResultError(log.Errorf("gh-ost detected an UPDATE to a unique key column this migration is iterating on. Such update is not supported. Column is `%s`", modifiedColumn))) + // return query, args, rowsDelta, log.Errorf("gh-ost detected an UPDATE to a unique key column this migration is iterating on. Such update is not supported. Column is `%s`", modifiedColumn) } query, sharedArgs, uniqueKeyArgs, err := sql.BuildDMLUpdateQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.SharedColumns, this.migrationContext.MappedSharedColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues()) + args := sqlutils.Args() args = append(args, sharedArgs...) args = append(args, uniqueKeyArgs...) - return query, args, 0, err + return buildResults(newDmlBuildResult(query, args, 0, err)) } } - return "", args, 0, fmt.Errorf("Unknown dml event type: %+v", dmlEvent.DML) + return buildResults(newDmlBuildResultError(fmt.Errorf("Unknown dml event type: %+v", dmlEvent.DML))) } // ApplyDMLEventQuery writes an entry to the ghost table, in response to an intercepted // original-table binlog event func (this *Applier) ApplyDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) error { - query, args, rowDelta, err := this.buildDMLEventQuery(dmlEvent) - if err != nil { - return err - } - // TODO The below is in preparation for transactional writes on the ghost tables. - // Such writes would be, for example: - // - prepended with sql_mode setup - // - prepended with time zone setup - // - prepended with SET SQL_LOG_BIN=0 - // - prepended with SET FK_CHECKS=0 - // etc. - // - // a known problem: https://github.com/golang/go/issues/9373 -- bitint unsigned values, not supported in database/sql - // is solved by silently converting unsigned bigints to string values. - // - - err = func() error { - tx, err := this.db.Begin() - if err != nil { - return err + for _, buildResult := range this.buildDMLEventQuery(dmlEvent) { + if buildResult.err != nil { + return buildResult.err } - sessionQuery := `SET + // TODO The below is in preparation for transactional writes on the ghost tables. + // Such writes would be, for example: + // - prepended with sql_mode setup + // - prepended with time zone setup + // - prepended with SET SQL_LOG_BIN=0 + // - prepended with SET FK_CHECKS=0 + // etc. + // + // a known problem: https://github.com/golang/go/issues/9373 -- bitint unsigned values, not supported in database/sql + // is solved by silently converting unsigned bigints to string values. + // + + err := func() error { + tx, err := this.db.Begin() + if err != nil { + return err + } + sessionQuery := `SET SESSION time_zone = '+00:00', sql_mode = CONCAT(@@session.sql_mode, ',STRICT_ALL_TABLES') ` - if _, err := tx.Exec(sessionQuery); err != nil { - return err - } - if _, err := tx.Exec(query, args...); err != nil { - return err - } - if err := tx.Commit(); err != nil { - return err - } - return nil - }() + if _, err := tx.Exec(sessionQuery); err != nil { + return err + } + if _, err := tx.Exec(buildResult.query, buildResult.args...); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return err + } + return nil + }() - if err != nil { - err = fmt.Errorf("%s; query=%s; args=%+v", err.Error(), query, args) - return log.Errore(err) - } - // no error - atomic.AddInt64(&this.migrationContext.TotalDMLEventsApplied, 1) - if this.migrationContext.CountTableRows { - atomic.AddInt64(&this.migrationContext.RowsDeltaEstimate, rowDelta) + if err != nil { + err = fmt.Errorf("%s; query=%s; args=%+v", err.Error(), buildResult.query, buildResult.args) + return log.Errore(err) + } + // no error + atomic.AddInt64(&this.migrationContext.TotalDMLEventsApplied, 1) + if this.migrationContext.CountTableRows { + atomic.AddInt64(&this.migrationContext.RowsDeltaEstimate, buildResult.rowsDelta) + } } return nil } @@ -1022,15 +1057,16 @@ func (this *Applier) ApplyDMLEventQueries(dmlEvents [](*binlog.BinlogDMLEvent)) return rollback(err) } for _, dmlEvent := range dmlEvents { - query, args, rowDelta, err := this.buildDMLEventQuery(dmlEvent) - if err != nil { - return rollback(err) + for _, buildResult := range this.buildDMLEventQuery(dmlEvent) { + if buildResult.err != nil { + return rollback(buildResult.err) + } + if _, err := tx.Exec(buildResult.query, buildResult.args...); err != nil { + err = fmt.Errorf("%s; query=%s; args=%+v", err.Error(), buildResult.query, buildResult.args) + return rollback(err) + } + totalDelta += buildResult.rowsDelta } - if _, err := tx.Exec(query, args...); err != nil { - err = fmt.Errorf("%s; query=%s; args=%+v", err.Error(), query, args) - return rollback(err) - } - totalDelta += rowDelta } if err := tx.Commit(); err != nil { return err diff --git a/localtests/fail-update-pk-column/expect_failure b/localtests/fail-update-pk-column/expect_failure deleted file mode 100644 index 1b73369..0000000 --- a/localtests/fail-update-pk-column/expect_failure +++ /dev/null @@ -1 +0,0 @@ -gh-ost detected an UPDATE to a unique key column this migration is iterating on From 203ea6c2cdf813d046aaafa6683ba7fe436655d9 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Mon, 20 Nov 2017 10:24:28 +0200 Subject: [PATCH 6/7] refactor, simplifid code --- go/logic/applier.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index a355796..ed3275f 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -46,10 +46,6 @@ func newDmlBuildResultError(err error) *dmlBuildResult { } } -func buildResults(args ...(*dmlBuildResult)) [](*dmlBuildResult) { - return args -} - // Applier connects and writes the the applier-server, which is the server where migration // happens. This is typically the master, but could be a replica when `--test-on-replica` or // `--execute-on-replica` are given. @@ -951,12 +947,12 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (result case binlog.DeleteDML: { query, uniqueKeyArgs, err := sql.BuildDMLDeleteQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.WhereColumnValues.AbstractValues()) - return buildResults(newDmlBuildResult(query, uniqueKeyArgs, -1, err)) + return append(results, newDmlBuildResult(query, uniqueKeyArgs, -1, err)) } case binlog.InsertDML: { query, sharedArgs, err := sql.BuildDMLInsertQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.SharedColumns, this.migrationContext.MappedSharedColumns, dmlEvent.NewColumnValues.AbstractValues()) - return buildResults(newDmlBuildResult(query, sharedArgs, 1, err)) + return append(results, newDmlBuildResult(query, sharedArgs, 1, err)) } case binlog.UpdateDML: { @@ -974,10 +970,10 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (result args := sqlutils.Args() args = append(args, sharedArgs...) args = append(args, uniqueKeyArgs...) - return buildResults(newDmlBuildResult(query, args, 0, err)) + return append(results, newDmlBuildResult(query, args, 0, err)) } } - return buildResults(newDmlBuildResultError(fmt.Errorf("Unknown dml event type: %+v", dmlEvent.DML))) + return append(results, newDmlBuildResultError(fmt.Errorf("Unknown dml event type: %+v", dmlEvent.DML))) } // ApplyDMLEventQuery writes an entry to the ghost table, in response to an intercepted From 58c381f0a08510f8520e6d365457466409b86048 Mon Sep 17 00:00:00 2001 From: Shlomi Noach Date: Tue, 21 Nov 2017 09:14:04 +0200 Subject: [PATCH 7/7] some cleanup --- go/logic/applier.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/go/logic/applier.go b/go/logic/applier.go index ed3275f..971f068 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -921,18 +921,14 @@ func (this *Applier) ShowStatusVariable(variableName string) (result int64, err return result, nil } +// updateModifiesUniqueKeyColumns checks whether a UPDATE DML event actually +// modifies values of the migration's unique key (the iterated key). This will call +// for special handling. func (this *Applier) updateModifiesUniqueKeyColumns(dmlEvent *binlog.BinlogDMLEvent) (modifiedColumn string, isModified bool) { - // log.Debugf("............ UPDATE") - // log.Debugf("............ UPDATE: %+v", this.migrationContext.UniqueKey.Columns.String()) - // log.Debugf("............ UPDATE: %+v", dmlEvent.WhereColumnValues.String()) - // log.Debugf("............ UPDATE: %+v", dmlEvent.NewColumnValues.String()) for _, column := range this.migrationContext.UniqueKey.Columns.Columns() { tableOrdinal := this.migrationContext.OriginalTableColumns.Ordinals[column.Name] whereColumnValue := dmlEvent.WhereColumnValues.AbstractValues()[tableOrdinal] newColumnValue := dmlEvent.NewColumnValues.AbstractValues()[tableOrdinal] - // log.Debugf("............ UPDATE: old value= %+v", whereColumnValue) - // log.Debugf("............ UPDATE: new value= %+v", newColumnValue) - // log.Debugf("............ UPDATE: equals? %+v", newColumnValue == whereColumnValue) if newColumnValue != whereColumnValue { return column.Name, true } @@ -956,15 +952,12 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (result } case binlog.UpdateDML: { - if modifiedColumn, isModified := this.updateModifiesUniqueKeyColumns(dmlEvent); isModified { - log.Debugf("---------------- Detected modifiedColumn: %+v. Will turn into DELETE+INSERT", modifiedColumn) + if _, isModified := this.updateModifiesUniqueKeyColumns(dmlEvent); isModified { dmlEvent.DML = binlog.DeleteDML results = append(results, this.buildDMLEventQuery(dmlEvent)...) dmlEvent.DML = binlog.InsertDML results = append(results, this.buildDMLEventQuery(dmlEvent)...) return results - // return buildResults(newDmlBuildResultError(log.Errorf("gh-ost detected an UPDATE to a unique key column this migration is iterating on. Such update is not supported. Column is `%s`", modifiedColumn))) - // return query, args, rowsDelta, log.Errorf("gh-ost detected an UPDATE to a unique key column this migration is iterating on. Such update is not supported. Column is `%s`", modifiedColumn) } query, sharedArgs, uniqueKeyArgs, err := sql.BuildDMLUpdateQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.SharedColumns, this.migrationContext.MappedSharedColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues()) args := sqlutils.Args()