Skip to content

Commit 698ae27

Browse files
jorendorffjorendorff
committed
Simplify by removing isUniqueKeyColumn now that it's no longer used.
1 parent 8a30693 commit 698ae27

5 files changed

Lines changed: 31 additions & 47 deletions

File tree

‎go/logic/applier.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,10 +1470,9 @@ func (this *Applier) buildDMLEventQuery(dmlEvent *binlog.BinlogDMLEvent) []*dmlB
14701470
results = append(results, this.buildDMLEventQuery(dmlEvent)...)
14711471
return results
14721472
}
1473-
query, sharedArgs, uniqueKeyArgs, err := this.dmlUpdateQueryBuilder.BuildQuery(dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues())
1473+
query, updateArgs, err := this.dmlUpdateQueryBuilder.BuildQuery(dmlEvent.NewColumnValues.AbstractValues(), dmlEvent.WhereColumnValues.AbstractValues())
14741474
args := sqlutils.Args()
1475-
args = append(args, sharedArgs...)
1476-
args = append(args, uniqueKeyArgs...)
1475+
args = append(args, updateArgs...)
14771476
return []*dmlBuildResult{newDmlBuildResult(query, args, 0, err)}
14781477
}
14791478
}

‎go/sql/builder.go‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,11 @@ func (b *CheckpointInsertQueryBuilder) BuildQuery(uniqueKeyArgs []interface{}) (
169169
}
170170
convertedArgs := make([]interface{}, 0, 2*b.uniqueKeyColumns.Len())
171171
for i, column := range b.uniqueKeyColumns.Columns() {
172-
minArg := column.convertArg(uniqueKeyArgs[i], true)
172+
minArg := column.convertArg(uniqueKeyArgs[i])
173173
convertedArgs = append(convertedArgs, minArg)
174174
}
175175
for i, column := range b.uniqueKeyColumns.Columns() {
176-
minArg := column.convertArg(uniqueKeyArgs[i+b.uniqueKeyColumns.Len()], true)
176+
minArg := column.convertArg(uniqueKeyArgs[i+b.uniqueKeyColumns.Len()])
177177
convertedArgs = append(convertedArgs, minArg)
178178
}
179179
return b.preparedStatement, convertedArgs, nil
@@ -533,7 +533,7 @@ func (b *DMLDeleteQueryBuilder) BuildQuery(args []interface{}) (string, []interf
533533
uniqueKeyArgs := make([]interface{}, 0, b.uniqueKeyColumns.Len())
534534
for _, column := range b.uniqueKeyColumns.Columns() {
535535
tableOrdinal := b.tableColumns.Ordinals[column.Name]
536-
arg := column.convertArg(args[tableOrdinal], true)
536+
arg := column.convertArg(args[tableOrdinal])
537537
uniqueKeyArgs = append(uniqueKeyArgs, arg)
538538
}
539539
return b.preparedStatement, uniqueKeyArgs, nil
@@ -595,7 +595,7 @@ func (b *DMLInsertQueryBuilder) BuildQuery(args []interface{}) (string, []interf
595595
sharedArgs := make([]interface{}, 0, b.sharedColumns.Len())
596596
for _, column := range b.sharedColumns.Columns() {
597597
tableOrdinal := b.tableColumns.Ordinals[column.Name]
598-
arg := column.convertArg(args[tableOrdinal], false)
598+
arg := column.convertArg(args[tableOrdinal])
599599
sharedArgs = append(sharedArgs, arg)
600600
}
601601
return b.preparedStatement, sharedArgs, nil
@@ -661,20 +661,18 @@ func NewDMLUpdateQueryBuilder(databaseName, tableName string, tableColumns, shar
661661

662662
// BuildQuery builds the arguments array for a DML event UPDATE query.
663663
// It returns the query string, the shared arguments array, and the unique key arguments array.
664-
func (b *DMLUpdateQueryBuilder) BuildQuery(valueArgs, whereArgs []interface{}) (string, []interface{}, []interface{}, error) {
665-
sharedArgs := make([]interface{}, 0, b.sharedColumns.Len())
664+
func (b *DMLUpdateQueryBuilder) BuildQuery(valueArgs, whereArgs []interface{}) (string, []interface{}, error) {
665+
args := make([]interface{}, 0, b.sharedColumns.Len()+b.uniqueKeyColumns.Len())
666666
for _, column := range b.sharedColumns.Columns() {
667667
tableOrdinal := b.tableColumns.Ordinals[column.Name]
668-
arg := column.convertArg(valueArgs[tableOrdinal], false)
669-
sharedArgs = append(sharedArgs, arg)
668+
arg := column.convertArg(valueArgs[tableOrdinal])
669+
args = append(args, arg)
670670
}
671-
672-
uniqueKeyArgs := make([]interface{}, 0, b.uniqueKeyColumns.Len())
673671
for _, column := range b.uniqueKeyColumns.Columns() {
674672
tableOrdinal := b.tableColumns.Ordinals[column.Name]
675-
arg := column.convertArg(whereArgs[tableOrdinal], true)
676-
uniqueKeyArgs = append(uniqueKeyArgs, arg)
673+
arg := column.convertArg(whereArgs[tableOrdinal])
674+
args = append(args, arg)
677675
}
678676

679-
return b.preparedStatement, sharedArgs, uniqueKeyArgs, nil
677+
return b.preparedStatement, args, nil
680678
}

‎go/sql/builder_test.go‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
647647
uniqueKeyColumns := NewColumnList([]string{"position"})
648648
builder, err := NewDMLUpdateQueryBuilder(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns)
649649
require.NoError(t, err)
650-
query, sharedArgs, uniqueKeyArgs, err := builder.BuildQuery(valueArgs, whereArgs)
650+
query, updateArgs, err := builder.BuildQuery(valueArgs, whereArgs)
651651
require.NoError(t, err)
652652
expected := `
653653
update /* gh-ost mydb.tbl */
@@ -657,15 +657,14 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
657657
((position = ?))
658658
`
659659
require.Equal(t, normalizeQuery(expected), normalizeQuery(query))
660-
require.Equal(t, []interface{}{3, "testname", 17, 23}, sharedArgs)
661-
require.Equal(t, []interface{}{17}, uniqueKeyArgs)
660+
require.Equal(t, []interface{}{3, "testname", 17, 23, 17}, updateArgs)
662661
}
663662
{
664663
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
665664
uniqueKeyColumns := NewColumnList([]string{"position", "name"})
666665
builder, err := NewDMLUpdateQueryBuilder(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns)
667666
require.NoError(t, err)
668-
query, sharedArgs, uniqueKeyArgs, err := builder.BuildQuery(valueArgs, whereArgs)
667+
query, updateArgs, err := builder.BuildQuery(valueArgs, whereArgs)
669668
require.NoError(t, err)
670669
expected := `
671670
update /* gh-ost mydb.tbl */
@@ -675,15 +674,14 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
675674
((position = ?) and (name = ?))
676675
`
677676
require.Equal(t, normalizeQuery(expected), normalizeQuery(query))
678-
require.Equal(t, []interface{}{3, "testname", 17, 23}, sharedArgs)
679-
require.Equal(t, []interface{}{17, "testname"}, uniqueKeyArgs)
677+
require.Equal(t, []interface{}{3, "testname", 17, 23, 17, "testname"}, updateArgs)
680678
}
681679
{
682680
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
683681
uniqueKeyColumns := NewColumnList([]string{"age"})
684682
builder, err := NewDMLUpdateQueryBuilder(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns)
685683
require.NoError(t, err)
686-
query, sharedArgs, uniqueKeyArgs, err := builder.BuildQuery(valueArgs, whereArgs)
684+
query, updateArgs, err := builder.BuildQuery(valueArgs, whereArgs)
687685
require.NoError(t, err)
688686
expected := `
689687
update /* gh-ost mydb.tbl */
@@ -693,15 +691,14 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
693691
((age = ?))
694692
`
695693
require.Equal(t, normalizeQuery(expected), normalizeQuery(query))
696-
require.Equal(t, []interface{}{3, "testname", 17, 23}, sharedArgs)
697-
require.Equal(t, []interface{}{56}, uniqueKeyArgs)
694+
require.Equal(t, []interface{}{3, "testname", 17, 23, 56}, updateArgs)
698695
}
699696
{
700697
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
701698
uniqueKeyColumns := NewColumnList([]string{"age", "position", "id", "name"})
702699
builder, err := NewDMLUpdateQueryBuilder(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns)
703700
require.NoError(t, err)
704-
query, sharedArgs, uniqueKeyArgs, err := builder.BuildQuery(valueArgs, whereArgs)
701+
query, updateArgs, err := builder.BuildQuery(valueArgs, whereArgs)
705702
require.NoError(t, err)
706703
expected := `
707704
update /* gh-ost mydb.tbl */
@@ -711,8 +708,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
711708
((age = ?) and (position = ?) and (id = ?) and (name = ?))
712709
`
713710
require.Equal(t, normalizeQuery(expected), normalizeQuery(query))
714-
require.Equal(t, []interface{}{3, "testname", 17, 23}, sharedArgs)
715-
require.Equal(t, []interface{}{56, 17, 3, "testname"}, uniqueKeyArgs)
711+
require.Equal(t, []interface{}{3, "testname", 17, 23, 56, 17, 3, "testname"}, updateArgs)
716712
}
717713
{
718714
sharedColumns := NewColumnList([]string{"id", "name", "position", "age"})
@@ -732,7 +728,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
732728
uniqueKeyColumns := NewColumnList([]string{"id"})
733729
builder, err := NewDMLUpdateQueryBuilder(databaseName, tableName, tableColumns, sharedColumns, mappedColumns, uniqueKeyColumns)
734730
require.NoError(t, err)
735-
query, sharedArgs, uniqueKeyArgs, err := builder.BuildQuery(valueArgs, whereArgs)
731+
query, updateArgs, err := builder.BuildQuery(valueArgs, whereArgs)
736732
require.NoError(t, err)
737733
expected := `
738734
update /* gh-ost mydb.tbl */
@@ -742,8 +738,7 @@ func TestBuildDMLUpdateQuery(t *testing.T) {
742738
((id = ?))
743739
`
744740
require.Equal(t, normalizeQuery(expected), normalizeQuery(query))
745-
require.Equal(t, []interface{}{3, "testname", 17, 23}, sharedArgs)
746-
require.Equal(t, []interface{}{3}, uniqueKeyArgs)
741+
require.Equal(t, []interface{}{3, "testname", 17, 23, 3}, updateArgs)
747742
}
748743
}
749744

@@ -759,7 +754,7 @@ func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) {
759754
require.NoError(t, err)
760755
{
761756
// test signed
762-
query, sharedArgs, uniqueKeyArgs, err := builder.BuildQuery(valueArgs, whereArgs)
757+
query, updateArgs, err := builder.BuildQuery(valueArgs, whereArgs)
763758
require.NoError(t, err)
764759
expected := `
765760
update /* gh-ost mydb.tbl */
@@ -769,14 +764,13 @@ func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) {
769764
((position = ?))
770765
`
771766
require.Equal(t, normalizeQuery(expected), normalizeQuery(query))
772-
require.Equal(t, []interface{}{3, "testname", int8(-17), int8(-2)}, sharedArgs)
773-
require.Equal(t, []interface{}{int8(-3)}, uniqueKeyArgs)
767+
require.Equal(t, []interface{}{3, "testname", int8(-17), int8(-2), int8(-3)}, updateArgs)
774768
}
775769
{
776770
// test unsigned
777771
sharedColumns.SetUnsigned("age")
778772
uniqueKeyColumns.SetUnsigned("position")
779-
query, sharedArgs, uniqueKeyArgs, err := builder.BuildQuery(valueArgs, whereArgs)
773+
query, updateArgs, err := builder.BuildQuery(valueArgs, whereArgs)
780774
require.NoError(t, err)
781775
expected := `
782776
update /* gh-ost mydb.tbl */
@@ -786,8 +780,7 @@ func TestBuildDMLUpdateQuerySignedUnsigned(t *testing.T) {
786780
((position = ?))
787781
`
788782
require.Equal(t, normalizeQuery(expected), normalizeQuery(query))
789-
require.Equal(t, []interface{}{3, "testname", int8(-17), uint8(254)}, sharedArgs)
790-
require.Equal(t, []interface{}{uint8(253)}, uniqueKeyArgs)
783+
require.Equal(t, []interface{}{3, "testname", int8(-17), uint8(254), uint8(253)}, updateArgs)
791784
}
792785
}
793786

‎go/sql/types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type Column struct {
5757
MySQLType string
5858
}
5959

60-
func (this *Column) convertArg(arg interface{}, isUniqueKeyColumn bool) interface{} {
60+
func (this *Column) convertArg(arg interface{}) interface{} {
6161
var arg2Bytes []byte
6262
if s, ok := arg.(string); ok {
6363
arg2Bytes = []byte(s)

‎go/sql/types_test.go‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestConvertArgCharsetDecoding(t *testing.T) {
6262
}
6363

6464
// Should decode []uint8
65-
str := col.convertArg(latin1Bytes, false)
65+
str := col.convertArg(latin1Bytes)
6666
require.Equal(t, "Garçon !", str)
6767
}
6868

@@ -85,20 +85,14 @@ func TestConvertArgBinaryColumnPadding(t *testing.T) {
8585
BinaryOctetLength: 20,
8686
}
8787

88-
// Test with isUniqueKeyColumn=false (the bug was that non-key columns weren't padded)
89-
result := col.convertArg(truncatedValue, false)
88+
result := col.convertArg(truncatedValue)
9089
resultBytes := []byte(result.(string))
9190

9291
require.Equal(t, 20, len(resultBytes), "binary column should be padded to declared length")
9392
// First 18 bytes should be unchanged
9493
require.Equal(t, truncatedValue, resultBytes[:18])
9594
// Last 2 bytes should be zeros
9695
require.Equal(t, []byte{0x00, 0x00}, resultBytes[18:])
97-
98-
// Test with isUniqueKeyColumn=true (this already worked before the fix)
99-
resultKey := col.convertArg(truncatedValue, true)
100-
resultKeyBytes := []byte(resultKey.(string))
101-
require.Equal(t, 20, len(resultKeyBytes), "binary key column should be padded to declared length")
10296
}
10397

10498
func TestConvertArgBinaryColumnNoPaddingWhenFull(t *testing.T) {
@@ -115,7 +109,7 @@ func TestConvertArgBinaryColumnNoPaddingWhenFull(t *testing.T) {
115109
BinaryOctetLength: 20,
116110
}
117111

118-
result := col.convertArg(fullValue, false)
112+
result := col.convertArg(fullValue)
119113
// When no padding needed, result may be []uint8 or string depending on code path
120114
var resultBytes []byte
121115
switch v := result.(type) {

0 commit comments

Comments
 (0)