Merge pull request vitessio#5688 from tinyspeck/fix-comparison-bug-fi…

…lepos-flavor

Fixes bug in filepos flavor

go/mysql/binlog_event_filepos.go

@@ -19,7 +19,6 @@ package mysql
 import (
  "encoding/binary"
  "fmt"
- "strconv"
 )
 
 // filePosBinlogEvent wraps a raw packet buffer and provides methods to examine
@@ -228,7 +227,7 @@ func newFilePosGTIDEvent(file string, pos int, timestamp uint32) filePosGTIDEven
  },
  gtid: filePosGTID{
  file: file,
- pos: strconv.Itoa(pos),
+ pos: pos,
  },
  }
 }

go/mysql/filepos_gtid.go

@@ -18,6 +18,7 @@ package mysql
 
 import (
  "fmt"
+ "strconv"
  "strings"
 )
 
@@ -31,9 +32,14 @@ func parseFilePosGTID(s string) (GTID, error) {
  return nil, fmt.Errorf("invalid FilePos GTID (%v): expecting file:pos", s)
  }
 
+ pos, err := strconv.Atoi(parts[1])
+ if err != nil {
+ return nil, fmt.Errorf("invalid FilePos GTID (%v): expecting pos to be an integer", s)
+ }
+
  return filePosGTID{
  file: parts[0],
- pos: parts[1],
+ pos: pos,
  }, nil
 }
 
@@ -48,12 +54,13 @@ func parseFilePosGTIDSet(s string) (GTIDSet, error) {
 
 // filePosGTID implements GTID.
 type filePosGTID struct {
- file, pos string
+ file string
+ pos int
 }
 
 // String implements GTID.String().
 func (gtid filePosGTID) String() string {
- return gtid.file + ":" + gtid.pos
+ return fmt.Sprintf("%s:%d", gtid.file, gtid.pos)
 }
 
 // Flavor implements GTID.Flavor().

go/mysql/filepos_gtid_test.go

@@ -0,0 +1,102 @@
+/*
+Copyright 2020 The Vitess Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package mysql
+
+import (
+ "testing"
+)
+
+func Test_filePosGTID_String(t *testing.T) {
+ type fields struct {
+ file string
+ pos int
+ }
+ tests := []struct {
+ name string
+ fields fields
+ want string
+ }{
+ {
+ "formats gtid correctly",
+ fields{file: "mysql-bin.166031", pos: 192394},
+ "mysql-bin.166031:192394",
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ gtid := filePosGTID{
+ file: tt.fields.file,
+ pos: tt.fields.pos,
+ }
+ if got := gtid.String(); got != tt.want {
+ t.Errorf("filePosGTID.String() = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}
+
+func Test_filePosGTID_ContainsGTID(t *testing.T) {
+ type fields struct {
+ file string
+ pos int
+ }
+ type args struct {
+ other GTID
+ }
+ tests := []struct {
+ name string
+ fields fields
+ args args
+ want bool
+ }{
+ {
+ "returns true when the position is equal",
+ fields{file: "testfile", pos: 1234},
+ args{other: filePosGTID{file: "testfile", pos: 1234}},
+ true,
+ },
+ {
+ "returns true when the position is less than equal",
+ fields{file: "testfile", pos: 1234},
+ args{other: filePosGTID{file: "testfile", pos: 1233}},
+ true,
+ },
+ {
+ "returns false when the position is less than equal",
+ fields{file: "testfile", pos: 1234},
+ args{other: filePosGTID{file: "testfile", pos: 1235}},
+ false,
+ },
+ {
+ "it uses integer value for comparison (it is not lexicographical order)",
+ fields{file: "testfile", pos: 99761227},
+ args{other: filePosGTID{file: "testfile", pos: 103939867}},
+ false,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ gtid := filePosGTID{
+ file: tt.fields.file,
+ pos: tt.fields.pos,
+ }
+ if got := gtid.ContainsGTID(tt.args.other); got != tt.want {
+ t.Errorf("filePosGTID.ContainsGTID() = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}

go/mysql/flavor_filepos.go

@@ -56,19 +56,29 @@ func (flv *filePosFlavor) masterGTIDSet(c *Conn) (GTIDSet, error) {
  if err != nil {
  return nil, err
  }
+ pos, err := strconv.Atoi(resultMap["Position"])
+ if err != nil {
+ return nil, fmt.Errorf("invalid FilePos GTID (%v): expecting pos to be an integer", resultMap["Position"])
+ }
+
  return filePosGTID{
  file: resultMap["File"],
- pos: resultMap["Position"],
+ pos: pos,
  }, nil
  }
 
  resultMap, err := resultToMap(qr)
  if err != nil {
  return nil, err
  }
+ pos, err := strconv.Atoi(resultMap["Exec_Master_Log_Pos"])
+ if err != nil {
+ return nil, fmt.Errorf("invalid FilePos GTID (%v): expecting pos to be an integer", resultMap["Exec_Master_Log_Pos"])
+ }
+
  return filePosGTID{
  file: resultMap["Relay_Master_Log_File"],
- pos: resultMap["Exec_Master_Log_Pos"],
+ pos: pos,
  }, nil
 }
 
@@ -91,13 +101,8 @@ func (flv *filePosFlavor) sendBinlogDumpCommand(c *Conn, slaveID uint32, startPo
  return fmt.Errorf("startPos.GTIDSet is wrong type - expected filePosGTID, got: %#v", startPos.GTIDSet)
  }
 
- pos, err := strconv.Atoi(rpos.pos)
- if err != nil {
- return fmt.Errorf("invalid position: %v", startPos.GTIDSet)
- }
  flv.file = rpos.file
-
- return c.WriteComBinlogDump(slaveID, rpos.file, uint32(pos), 0)
+ return c.WriteComBinlogDump(slaveID, rpos.file, uint32(rpos.pos), 0)
 }
 
 // readBinlogEvent is part of the Flavor interface.
@@ -212,9 +217,14 @@ func (flv *filePosFlavor) status(c *Conn) (SlaveStatus, error) {
  }
 
  status := parseSlaveStatus(resultMap)
+ pos, err := strconv.Atoi(resultMap["Exec_Master_Log_Pos"])
+ if err != nil {
+ return SlaveStatus{}, fmt.Errorf("invalid FilePos GTID (%v): expecting pos to be an integer", resultMap["Exec_Master_Log_Pos"])
+ }
+
  status.Position.GTIDSet = filePosGTID{
  file: resultMap["Relay_Master_Log_File"],
- pos: resultMap["Exec_Master_Log_Pos"],
+ pos: pos,
  }
  return status, nil
 }
@@ -231,10 +241,10 @@ func (flv *filePosFlavor) waitUntilPositionCommand(ctx context.Context, pos Posi
  if timeout <= 0 {
  return "", fmt.Errorf("timed out waiting for position %v", pos)
  }
- return fmt.Sprintf("SELECT MASTER_POS_WAIT('%s', %s, %.6f)", filePosPos.file, filePosPos.pos, timeout.Seconds()), nil
+ return fmt.Sprintf("SELECT MASTER_POS_WAIT('%s', %d, %.6f)", filePosPos.file, filePosPos.pos, timeout.Seconds()), nil
  }
 
- return fmt.Sprintf("SELECT MASTER_POS_WAIT('%s', %s)", filePosPos.file, filePosPos.pos), nil
+ return fmt.Sprintf("SELECT MASTER_POS_WAIT('%s', %d)", filePosPos.file, filePosPos.pos), nil
 }
 
 func (*filePosFlavor) startSlaveUntilAfter(pos Position) string {