From 1371618b7fdd492b2fc7581d8ae40d95aef7e1c4 Mon Sep 17 00:00:00 2001 From: lewgun Date: Thu, 10 Jan 2019 17:09:54 +0800 Subject: [PATCH 1/9] fix typo --- pkg/check/binlog.go | 2 +- pkg/check/privilege.go | 2 +- pkg/check/table_structure.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/check/binlog.go b/pkg/check/binlog.go index ac7b6f723..dde89dce6 100644 --- a/pkg/check/binlog.go +++ b/pkg/check/binlog.go @@ -37,7 +37,7 @@ func NewMySQLBinlogEnableChecker(db *sql.DB, dbinfo *dbutil.DBConfig) Checker { func (pc *MySQLBinlogEnableChecker) Check(ctx context.Context) *Result { result := &Result{ Name: pc.Name(), - Desc: "check whether mysql binlog is enable", + Desc: "check whether mysql binlog is enabled", State: StateFailure, Extra: fmt.Sprintf("address of db instance - %s:%d", pc.dbinfo.Host, pc.dbinfo.Port), } diff --git a/pkg/check/privilege.go b/pkg/check/privilege.go index c8694cb48..993b984cd 100644 --- a/pkg/check/privilege.go +++ b/pkg/check/privilege.go @@ -69,7 +69,7 @@ func (pc *SourcePrivilegeChecker) Check(ctx context.Context) *Result { } grantStmt, ok := node.(*ast.GrantStmt) if !ok { - result.ErrorMsg = fmt.Sprintf("%s is not grant stament", grants[0]) + result.ErrorMsg = fmt.Sprintf("%s is not grant statment", grants[0]) return result } diff --git a/pkg/check/table_structure.go b/pkg/check/table_structure.go index e49aa7925..636b7bc3c 100644 --- a/pkg/check/table_structure.go +++ b/pkg/check/table_structure.go @@ -56,7 +56,7 @@ func (o *incompatibilityOption) String() string { return text.String() } -// TablesChecker checks compatibility of table structures, there are differents between MySQL and TiDB. +// TablesChecker checks compatibility of table structures, there are differences between MySQL and TiDB. // In generally we need to check definitions of columns, constraints and table options. // Because of the early TiDB engineering design, we did not have a complete list of check items, which are all based on experience now. type TablesChecker struct { From 98e20f6345b493d13dd3e48c3cd7c6ce7103a507 Mon Sep 17 00:00:00 2001 From: lewgun Date: Thu, 10 Jan 2019 17:12:02 +0800 Subject: [PATCH 2/9] remove the redundant parentheses --- pkg/check/check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/check/check.go b/pkg/check/check.go index c10cdc5d8..bc0c1d73d 100644 --- a/pkg/check/check.go +++ b/pkg/check/check.go @@ -101,7 +101,7 @@ func Do(ctx context.Context, checkers []Checker) (*Results, error) { } // if total == successful + warning + failed, it's finished - finished = (total == successful+warning+failed) + finished = total == successful+warning+failed results.Results = append(results.Results, result) if finished { From bf55d1f038cb4a8ea2dbf30e70c048d7f97b3d12 Mon Sep 17 00:00:00 2001 From: lewgun Date: Thu, 10 Jan 2019 21:43:10 +0800 Subject: [PATCH 3/9] adjust a few error messages --- pkg/column-mapping/column.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/column-mapping/column.go b/pkg/column-mapping/column.go index b5283af3c..91530247b 100644 --- a/pkg/column-mapping/column.go +++ b/pkg/column-mapping/column.go @@ -465,7 +465,7 @@ func partitionID(info *mappingInfo, vals []interface{}) ([]interface{}, error) { } if originID >= maxOriginID || originID < 0 { - return nil, errors.NotValidf("id must less than %d, bigger or equal than 0, but get %d", maxOriginID, originID) + return nil, errors.NotValidf("id must less than %d, greater than or equal to 0, but got %d", maxOriginID, originID) } originID = int64(info.instanceID | info.schemaID | info.tableID | originID) @@ -507,13 +507,13 @@ func computePartitionID(schema, table string, rule *Rule) (instanceID int64, sch func computeID(name string, prefix string, bitSize int, shiftCount uint) (int64, error) { if len(prefix) >= len(name) || prefix != name[:len(prefix)] { - return 0, errors.NotValidf("%s is not prefix of %s", prefix, name) + return 0, errors.NotValidf("%s is not the prefix of %s", prefix, name) } idStr := name[len(prefix):] id, err := strconv.ParseUint(idStr, 10, bitSize) if err != nil { - return 0, errors.NotValidf("suffix of %s is not int64", idStr) + return 0, errors.NotValidf("suffix of %s's data type is not int64", idStr) } return int64(id << shiftCount), nil From 77c56d74e8b0f924822a886957bd7762c140e53e Mon Sep 17 00:00:00 2001 From: lewgun Date: Tue, 15 Jan 2019 09:03:37 +0800 Subject: [PATCH 4/9] adjust a few error messages --- pkg/column-mapping/column.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/column-mapping/column.go b/pkg/column-mapping/column.go index 91530247b..fb96af54e 100644 --- a/pkg/column-mapping/column.go +++ b/pkg/column-mapping/column.go @@ -513,7 +513,7 @@ func computeID(name string, prefix string, bitSize int, shiftCount uint) (int64, idStr := name[len(prefix):] id, err := strconv.ParseUint(idStr, 10, bitSize) if err != nil { - return 0, errors.NotValidf("suffix of %s's data type is not int64", idStr) + return 0, errors.NotValidf("the suffix of %s can't be converted to int64", idStr) } return int64(id << shiftCount), nil From 7bba47d7356d382a19b944b65308e26efb3a4a04 Mon Sep 17 00:00:00 2001 From: lewgun Date: Tue, 15 Jan 2019 11:26:29 +0800 Subject: [PATCH 5/9] fix typos & remove the redundant parentheses & more --- pkg/dbutil/index.go | 4 ++-- pkg/dbutil/query.go | 2 +- pkg/dbutil/table.go | 2 +- pkg/ddl-checker/executable_checker.go | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/dbutil/index.go b/pkg/dbutil/index.go index db5ce02a5..d2de10aa5 100644 --- a/pkg/dbutil/index.go +++ b/pkg/dbutil/index.go @@ -46,7 +46,7 @@ func ShowIndex(ctx context.Context, db *sql.DB, schemaName string, table string) if err1 != nil { return nil, errors.Trace(err1) } - seqInINdex, err1 := strconv.Atoi(string(fields["Seq_in_index"])) + seqInIndex, err1 := strconv.Atoi(string(fields["Seq_in_index"])) if err != nil { return nil, errors.Trace(err1) } @@ -59,7 +59,7 @@ func ShowIndex(ctx context.Context, db *sql.DB, schemaName string, table string) NoneUnique: string(fields["Non_unique"]) == "1", KeyName: string(fields["Key_name"]), ColumnName: string(fields["Column_name"]), - SeqInIndex: seqInINdex, + SeqInIndex: seqInIndex, Cardinality: cardinality, } indices = append(indices, index) diff --git a/pkg/dbutil/query.go b/pkg/dbutil/query.go index 75c9ead5c..92984bdd5 100644 --- a/pkg/dbutil/query.go +++ b/pkg/dbutil/query.go @@ -49,7 +49,7 @@ func ScanRow(rows *sql.Rows) (map[string][]byte, map[string]bool, error) { null := make(map[string]bool) for i := range colVals { result[cols[i]] = colVals[i] - null[cols[i]] = (colVals[i] == nil) + null[cols[i]] = colVals[i] == nil } return result, null, nil diff --git a/pkg/dbutil/table.go b/pkg/dbutil/table.go index 94445def1..86de9ccb3 100644 --- a/pkg/dbutil/table.go +++ b/pkg/dbutil/table.go @@ -144,7 +144,7 @@ func columnDefToCol(offset int, colDef *ast.ColumnDef) (*model.ColumnInfo, []*as return column, constraints } -// BuildTableInfo builds table infomation using column constraints. +// BuildTableInfo builds table information using column constraints. func BuildTableInfo(tableName model.CIStr, columns []*model.ColumnInfo, constraints []*ast.Constraint) (tbInfo *model.TableInfo, err error) { tbInfo = &model.TableInfo{ Name: tableName, diff --git a/pkg/ddl-checker/executable_checker.go b/pkg/ddl-checker/executable_checker.go index d6a66f129..963d60ce8 100644 --- a/pkg/ddl-checker/executable_checker.go +++ b/pkg/ddl-checker/executable_checker.go @@ -38,15 +38,15 @@ func NewExecutableChecker() (*ExecutableChecker, error) { logutil.InitLogger(&logutil.LogConfig{ Level: "error", }) - mocktikv, err := mockstore.NewMockTikvStore() + mockTikv, err := mockstore.NewMockTikvStore() if err != nil { return nil, errors.Trace(err) } - _, err = session.BootstrapSession(mocktikv) + _, err = session.BootstrapSession(mockTikv) if err != nil { return nil, errors.Trace(err) } - session, err := session.CreateSession4Test(mocktikv) + session, err := session.CreateSession4Test(mockTikv) if err != nil { return nil, errors.Trace(err) } From a49ecbfa43092157ecd0980ea14d50992e4ab0e2 Mon Sep 17 00:00:00 2001 From: lewgun Date: Wed, 16 Jan 2019 22:04:10 +0800 Subject: [PATCH 6/9] simplify the if/else logic --- pkg/diff/chunk.go | 25 ++++++++++++++----------- pkg/diff/diff.go | 31 ++++++++++++++++++------------- pkg/diff/merge.go | 23 ++++++++++++++--------- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/pkg/diff/chunk.go b/pkg/diff/chunk.go index f51ede381..5731b34dc 100644 --- a/pkg/diff/chunk.go +++ b/pkg/diff/chunk.go @@ -281,27 +281,30 @@ func GenerateCheckJob(table *TableInstance, splitField, limits string, chunkSize chunks = chunks[1:] args := make([]interface{}, 0, 2) - var condition1, condition2 string + var ( + condition1 = "TRUE" + condition2 = "TRUE" + ) if !chunk.noBegin { + format := "`%s`%s > ?" if chunk.containBegin { - condition1 = fmt.Sprintf("`%s`%s >= ?", column.Name, collation) - } else { - condition1 = fmt.Sprintf("`%s`%s > ?", column.Name, collation) + format = "`%s`%s >= ?" } + + condition1 = fmt.Sprintf(format, column.Name, collation) args = append(args, chunk.begin) - } else { - condition1 = "TRUE" } + if !chunk.noEnd { + format := "`%s`%s < ?" if chunk.containEnd { - condition2 = fmt.Sprintf("`%s`%s <= ?", column.Name, collation) - } else { - condition2 = fmt.Sprintf("`%s`%s < ?", column.Name, collation) + format = "`%s`%s <= ?" } + + condition2 = fmt.Sprintf(format, column.Name, collation) args = append(args, chunk.end) - } else { - condition2 = "TRUE" } + where := fmt.Sprintf("(%s AND %s AND %s)", condition1, condition2, limits) log.Debugf("%s.%s create dump job, where: %s, begin: %v, end: %v", table.Schema, table.Table, where, chunk.begin, chunk.end) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index c17bf9d07..4254d6a82 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -506,29 +506,34 @@ func compareData(map1, map2 map[string][]byte, null1, null2 map[string]bool, ord return false, 0, errors.Errorf("don't have key %s", col.Name.O) } if needQuotes(col.FieldType) { - if string(data1) > string(data2) { - cmp = 1 - break - } else if string(data1) < string(data2) { - cmp = -1 - break - } else { + + strData1 := string(data1) + strData2 := string(data2) + + if strData1 == strData2 { continue } + + cmp = -1 + if strData1 > strData2 { + cmp = 1 + } + break + } else { num1, err1 := strconv.ParseFloat(string(data1), 64) num2, err2 := strconv.ParseFloat(string(data2), 64) if err1 != nil || err2 != nil { return false, 0, errors.Errorf("convert %s, %s to float failed, err1: %v, err2: %v", string(data1), string(data2), err1, err2) } + + if num1 == num2 { + continue + } + + cmp = -1 if num1 > num2 { cmp = 1 - break - } else if num1 < num2 { - cmp = -1 - break - } else { - continue } } } diff --git a/pkg/diff/merge.go b/pkg/diff/merge.go index 610ec8656..f84456b80 100644 --- a/pkg/diff/merge.go +++ b/pkg/diff/merge.go @@ -41,11 +41,10 @@ func (r RowDatas) Less(i, j int) bool { data1 = r.Rows[i].Data[col.Name.O] data2 = r.Rows[j].Data[col.Name.O] if needQuotes(col.FieldType) { - if string(data1) > string(data2) { - return false - } else if string(data1) < string(data2) { - return true - } else { + strData1 := string(data1) + strData2 := string(data2) + + if strData1 == strData2 { // `NULL` is less than "" if r.Rows[i].Null[col.Name.O] { return true @@ -55,19 +54,25 @@ func (r RowDatas) Less(i, j int) bool { } continue } + if strData1 > strData2 { + return false + } + return true } else { num1, err1 := strconv.ParseFloat(string(data1), 64) num2, err2 := strconv.ParseFloat(string(data2), 64) if err1 != nil || err2 != nil { log.Fatalf("convert %s, %s to float failed, err1: %v, err2: %v", string(data1), string(data2), err1, err2) } + + if num1 == num2 { + continue + } if num1 > num2 { return false - } else if num1 < num2 { - return true - } else { - continue } + return true + } } From 758258e4a6c483640f4a01f10740d0dd0f0fd5be Mon Sep 17 00:00:00 2001 From: lewgun Date: Wed, 16 Jan 2019 22:08:17 +0800 Subject: [PATCH 7/9] add the missing 'break' --- pkg/diff/diff.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 4254d6a82..b758032a9 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -535,6 +535,7 @@ func compareData(map1, map2 map[string][]byte, null1, null2 map[string]bool, ord if num1 > num2 { cmp = 1 } + break } } From ca0960750c0b1d18af7817edaf0c328607adde60 Mon Sep 17 00:00:00 2001 From: lewgun Date: Sun, 20 Jan 2019 10:28:07 +0800 Subject: [PATCH 8/9] make the golint happy --- pkg/diff/merge.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/pkg/diff/merge.go b/pkg/diff/merge.go index f84456b80..7e4894f1c 100644 --- a/pkg/diff/merge.go +++ b/pkg/diff/merge.go @@ -58,22 +58,21 @@ func (r RowDatas) Less(i, j int) bool { return false } return true - } else { - num1, err1 := strconv.ParseFloat(string(data1), 64) - num2, err2 := strconv.ParseFloat(string(data2), 64) - if err1 != nil || err2 != nil { - log.Fatalf("convert %s, %s to float failed, err1: %v, err2: %v", string(data1), string(data2), err1, err2) - } - - if num1 == num2 { - continue - } - if num1 > num2 { - return false - } - return true + } + num1, err1 := strconv.ParseFloat(string(data1), 64) + num2, err2 := strconv.ParseFloat(string(data2), 64) + if err1 != nil || err2 != nil { + log.Fatalf("convert %s, %s to float failed, err1: %v, err2: %v", string(data1), string(data2), err1, err2) + } + if num1 == num2 { + continue } + if num1 > num2 { + return false + } + return true + } return true From 923c7b8f796f2808ec3477342993da4f132164e9 Mon Sep 17 00:00:00 2001 From: lewgun Date: Fri, 25 Jan 2019 10:33:07 +0800 Subject: [PATCH 9/9] compare the two strings' length first --- pkg/diff/diff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index b758032a9..b7a4beb09 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -510,7 +510,7 @@ func compareData(map1, map2 map[string][]byte, null1, null2 map[string]bool, ord strData1 := string(data1) strData2 := string(data2) - if strData1 == strData2 { + if len(strData1) == len(strData2) && strData1 == strData2 { continue }