Skip to content

Commit 1376f0a

Browse files
author
Shlomi Noach
committed
fixed UPDATE dml on renamed column
1 parent 36a2863 commit 1376f0a

File tree

4 files changed

+37
-20
lines changed

4 files changed

+37
-20
lines changed

‎go/logic/applier.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ func (this *Applier) ValidateOrDropExistingTables() error {
107107
}
108108
}
109109
if this.tableExists(this.migrationContext.GetGhostTableName()) {
110-
return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-ghost-table to force dropping it", sql.EscapeName(this.migrationContext.GetGhostTableName()))
110+
return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-ghost-table to force dropping it, though I really prefer that you drop it or rename it away", sql.EscapeName(this.migrationContext.GetGhostTableName()))
111111
}
112112
if this.migrationContext.InitiallyDropOldTable {
113113
if err := this.DropOldTable(); err != nil {
114114
return err
115115
}
116116
}
117117
if this.tableExists(this.migrationContext.GetOldTableName()) {
118-
return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-old-table to force dropping it", sql.EscapeName(this.migrationContext.GetOldTableName()))
118+
return fmt.Errorf("Table %s already exists. Panicking. Use --initially-drop-old-table to force dropping it, though I really prefer that you drop it or rename it away", sql.EscapeName(this.migrationContext.GetOldTableName()))
119119
}
120120

121121
return nil
@@ -837,7 +837,7 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) (query
837837
}
838838
case binlog.UpdateDML:
839839
{
840-
query, sharedArgs, uniqueKeyArgs, err := sql.BuildDMLUpdateQuery(dmlEvent.DatabaseName, this.migrationContext.GetGhostTableName(), this.migrationContext.OriginalTableColumns, this.migrationContext.MappedSharedColumns, &this.migrationContext.UniqueKey.Columns, dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues())
840+
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())
841841
args = append(args, sharedArgs...)
842842
args = append(args, uniqueKeyArgs...)
843843
return query, args, 0, err
@@ -853,7 +853,6 @@ func (this *Applier) ApplyDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) error {
853853
if err != nil {
854854
return err
855855
}
856-
857856
// TODO The below is in preparation for transactional writes on the ghost tables.
858857
// Such writes would be, for example:
859858
// - prepended with sql_mode setup

‎go/logic/migrator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ func (this *Migrator) validateStatement() (err error) {
354354
if this.parser.HasNonTrivialRenames() && !this.migrationContext.SkipRenamedColumns {
355355
this.migrationContext.ColumnRenameMap = this.parser.GetNonTrivialRenames()
356356
if !this.migrationContext.ApproveRenamedColumns {
357-
return fmt.Errorf("Alter statement has column(s) renamed. gh-ost suspects the following renames: %v; but to proceed you must approve via `--approve-renamed-columns` (or you can skip renamed columns via `--skip-renamed-columns`)", this.parser.GetNonTrivialRenames())
357+
return fmt.Errorf("gh-ost believes the ALTER statement renames columns, as follows: %v; as precation, you are asked to confirm gh-ost is correct, and provide with `--approve-renamed-columns`, and we're all happy. Or you can skip renamed columns via `--skip-renamed-columns`, in which case column data may be lost", this.parser.GetNonTrivialRenames())
358358
}
359359
log.Infof("Alter statement has column(s) renamed. gh-ost finds the following renames: %v; --approve-renamed-columns is given and so migration proceeds.", this.parser.GetNonTrivialRenames())
360360
}

‎go/sql/builder.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ func BuildDMLInsertQuery(databaseName, tableName string, tableColumns, sharedCol
393393
return result, sharedArgs, nil
394394
}
395395

396-
func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedColumns, uniqueKeyColumns *ColumnList, valueArgs, whereArgs []interface{}) (result string, sharedArgs, uniqueKeyArgs []interface{}, err error) {
396+
func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedColumns, mappedSharedColumns, uniqueKeyColumns *ColumnList, valueArgs, whereArgs []interface{}) (result string, sharedArgs, uniqueKeyArgs []interface{}, err error) {
397397
if len(valueArgs) != tableColumns.Len() {
398398
return result, sharedArgs, uniqueKeyArgs, fmt.Errorf("value args count differs from table column count in BuildDMLUpdateQuery")
399399
}
@@ -415,9 +415,10 @@ func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedCol
415415
databaseName = EscapeName(databaseName)
416416
tableName = EscapeName(tableName)
417417

418-
for _, column := range sharedColumns.Names {
418+
for i, column := range sharedColumns.Names {
419+
mappedColumn := mappedSharedColumns.Names[i]
419420
tableOrdinal := tableColumns.Ordinals[column]
420-
arg := fixArgType(valueArgs[tableOrdinal], sharedColumns.IsUnsigned(column))
421+
arg := fixArgType(valueArgs[tableOrdinal], mappedSharedColumns.IsUnsigned(mappedColumn))
421422
sharedArgs = append(sharedArgs, arg)
422423
}
423424

@@ -427,11 +428,11 @@ func BuildDMLUpdateQuery(databaseName, tableName string, tableColumns, sharedCol
427428
uniqueKeyArgs = append(uniqueKeyArgs, arg)
428429
}
429430

430-
sharedColumnNames := duplicateNames(sharedColumns.Names)
431-
for i := range sharedColumnNames {
432-
sharedColumnNames[i] = EscapeName(sharedColumnNames[i])
431+
mappedSharedColumnNames := duplicateNames(mappedSharedColumns.Names)
432+
for i := range mappedSharedColumnNames {
433+
mappedSharedColumnNames[i] = EscapeName(mappedSharedColumnNames[i])
433434
}
434-
setClause, err := BuildSetPreparedClause(sharedColumnNames)
435+
setClause, err := BuildSetPreparedClause(mappedSharedColumnNames)
435436

436437
equalsComparison, err := BuildEqualsPreparedComparison(uniqueKeyColumns.Names)
437438
result = fmt.Sprintf(`

‎go/sql/builder_test.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
544544
{
545545
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
546546
uniqueKeyColumns := NewColumnList([]string{"position"})
547-
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
547+
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
548548
test.S(t).ExpectNil(err)
549549
expected := `
550550
update /* gh-ost mydb.tbl */
@@ -560,7 +560,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
560560
{
561561
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
562562
uniqueKeyColumns := NewColumnList([]string{"position", "name"})
563-
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
563+
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
564564
test.S(t).ExpectNil(err)
565565
expected := `
566566
update /* gh-ost mydb.tbl */
@@ -576,7 +576,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
576576
{
577577
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
578578
uniqueKeyColumns := NewColumnList([]string{"age"})
579-
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
579+
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
580580
test.S(t).ExpectNil(err)
581581
expected := `
582582
update /* gh-ost mydb.tbl */
@@ -592,7 +592,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
592592
{
593593
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
594594
uniqueKeyColumns := NewColumnList([]string{"age", "position", "id", "name"})
595-
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
595+
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
596596
test.S(t).ExpectNil(err)
597597
expected := `
598598
update /* gh-ost mydb.tbl */
@@ -608,15 +608,32 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
608608
{
609609
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
610610
uniqueKeyColumns := NewColumnList([]string{"age", "surprise"})
611-
_, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
611+
_, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
612612
test.S(t).ExpectNotNil(err)
613613
}
614614
{
615615
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
616616
uniqueKeyColumns := NewColumnList([]string{})
617-
_, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
617+
_, _, _, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
618618
test.S(t).ExpectNotNil(err)
619619
}
620+
{
621+
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
622+
mappedColumns := NewColumnList([]string{"id", "name", "role", "age"})
623+
uniqueKeyColumns := NewColumnList([]string{"id"})
624+
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, mappedColumns, uniqueKeyColumns, valueArgs, whereArgs)
625+
test.S(t).ExpectNil(err)
626+
expected := `
627+
update /* gh-ost mydb.tbl */
628+
mydb.tbl
629+
set id=?, name=?, role=?, age=?
630+
where
631+
((id = ?))
632+
`
633+
test.S(t).ExpectEquals(normalizeQuery(query), normalizeQuery(expected))
634+
test.S(t).ExpectTrue(reflect.DeepEqual(sharedArgs, []interface{}{3, "testname", 17, 23}))
635+
test.S(t).ExpectTrue(reflect.DeepEqual(uniqueKeyArgs, []interface{}{3}))
636+
}
620637
}
621638

622639
func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) {
@@ -629,7 +646,7 @@ func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) {
629646
uniqueKeyColumns := NewColumnList([]string{"position"})
630647
{
631648
// test signed
632-
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
649+
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
633650
test.S(t).ExpectNil(err)
634651
expected := `
635652
update /* gh-ost mydb.tbl */
@@ -646,7 +663,7 @@ func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) {
646663
// test unsigned
647664
sharedColumns.SetUnsigned("age")
648665
uniqueKeyColumns.SetUnsigned("position")
649-
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
666+
query, sharedArgs, uniqueKeyArgs, err := BuildDMLUpdateQuery(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns, valueArgs, whereArgs)
650667
test.S(t).ExpectNil(err)
651668
expected := `
652669
update /* gh-ost mydb.tbl */

0 commit comments

Comments
 (0)