From 3bb6889b5d18de37cb5fc27172b875c7936f2a31 Mon Sep 17 00:00:00 2001 From: crazycs Date: Fri, 12 Oct 2018 23:32:11 +0800 Subject: [PATCH 1/3] admin: fix admin check byte compare bug --- executor/executor_test.go | 9 +++++++++ util/admin/admin.go | 5 +++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index dcbfec3fbd61b..454aa4fe32529 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -301,6 +301,15 @@ func (s *testSuite) TestAdmin(c *C) { tk.MustExec("alter table t1 add index idx_i(i);") tk.MustExec("alter table t1 add index idx_m(a,c,d,e,f,g,h,i,j);") tk.MustExec("admin check table t1;") + + tk.MustExec("drop table if exists t1;") + tk.MustExec("CREATE TABLE t1 (c1 int);") + tk.MustExec("INSERT INTO t1 SET c1 = 1;") + tk.MustExec("ALTER TABLE t1 ADD COLUMN cc1 CHAR(36) NULL DEFAULT '';") + tk.MustExec("ALTER TABLE t1 ADD COLUMN cc2 VARCHAR(36) NULL DEFAULT ''") + tk.MustExec("ALTER TABLE t1 ADD INDEX idx1 (cc1);") + tk.MustExec("ALTER TABLE t1 ADD INDEX idx2 (cc2);") + tk.MustExec("admin check table t1;") } func (s *testSuite) fillData(tk *testkit.TestKit, table string) { diff --git a/util/admin/admin.go b/util/admin/admin.go index 3e767f73021d3..98ae13811badd 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -391,12 +391,13 @@ func compareDatumSlice(sc *stmtctx.StatementContext, val1s, val2s []types.Datum) return false } for i, v := range val1s { - if v.Kind() == types.KindMysqlDecimal { + switch v.Kind() { + case types.KindMysqlDecimal, types.KindBytes: res, err := v.CompareDatum(sc, &val2s[i]) if err != nil || res != 0 { return false } - } else { + default: if !reflect.DeepEqual(v, val2s[i]) { return false } From 1dec0c787f9ea2ab62d89bd67b61731bf3406fa9 Mon Sep 17 00:00:00 2001 From: crazycs Date: Mon, 15 Oct 2018 13:06:19 +0800 Subject: [PATCH 2/3] admin: remove reflect deepEqual --- types/datum_test.go | 38 ++++++++++++++++++++++++++++++++++++++ util/admin/admin.go | 17 +++++------------ 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/types/datum_test.go b/types/datum_test.go index bbb66780aae3f..f2317a77792a6 100644 --- a/types/datum_test.go +++ b/types/datum_test.go @@ -14,6 +14,8 @@ package types import ( + "reflect" + "testing" "time" . "github.com/pingcap/check" @@ -362,3 +364,39 @@ func (ts *testDatumSuite) TestCopyDatum(c *C) { } } } + +func prepareCompareDatums() ([]Datum, []Datum) { + vals := make([]Datum, 0, 5) + vals = append(vals, NewIntDatum(1)) + vals = append(vals, NewFloat64Datum(1.23)) + vals = append(vals, NewStringDatum("abcde")) + vals = append(vals, NewDecimalDatum(NewDecFromStringForTest("1.2345"))) + vals = append(vals, NewTimeDatum(Time{Time: FromGoTime(time.Date(2018, 3, 8, 16, 1, 0, 315313000, time.UTC)), Fsp: 6, Type: mysql.TypeTimestamp})) + + vals1 := make([]Datum, 0, 5) + vals1 = append(vals1, NewIntDatum(1)) + vals1 = append(vals1, NewFloat64Datum(1.23)) + vals1 = append(vals1, NewStringDatum("abcde")) + vals1 = append(vals1, NewDecimalDatum(NewDecFromStringForTest("1.2345"))) + vals1 = append(vals1, NewTimeDatum(Time{Time: FromGoTime(time.Date(2018, 3, 8, 16, 1, 0, 315313000, time.UTC)), Fsp: 6, Type: mysql.TypeTimestamp})) + return vals, vals1 +} + +func BenchmarkCompareDatum(b *testing.B) { + vals, vals1 := prepareCompareDatums() + sc := new(stmtctx.StatementContext) + b.ResetTimer() + for i := 0; i < b.N; i++ { + for j, v := range vals { + v.CompareDatum(sc, &vals1[j]) + } + } +} + +func BenchmarkCompareDatumByReflect(b *testing.B) { + vals, vals1 := prepareCompareDatums() + b.ResetTimer() + for i := 0; i < b.N; i++ { + reflect.DeepEqual(vals, vals1) + } +} diff --git a/util/admin/admin.go b/util/admin/admin.go index 98ae13811badd..b536c13f95493 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -16,7 +16,6 @@ package admin import ( "fmt" "io" - "reflect" "sort" "github.com/pingcap/tidb/expression" @@ -391,16 +390,9 @@ func compareDatumSlice(sc *stmtctx.StatementContext, val1s, val2s []types.Datum) return false } for i, v := range val1s { - switch v.Kind() { - case types.KindMysqlDecimal, types.KindBytes: - res, err := v.CompareDatum(sc, &val2s[i]) - if err != nil || res != 0 { - return false - } - default: - if !reflect.DeepEqual(v, val2s[i]) { - return false - } + res, err := v.CompareDatum(sc, &val2s[i]) + if err != nil || res != 0 { + return false } } return true @@ -524,6 +516,7 @@ func CompareTableRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table. } startKey := t.RecordKey(0) + sc := new(stmtctx.StatementContext) filterFunc := func(h int64, vals []types.Datum, cols []*table.Column) (bool, error) { vals2, ok := m[h] if !ok { @@ -535,7 +528,7 @@ func CompareTableRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table. return true, nil } - if !reflect.DeepEqual(vals, vals2) { + if !compareDatumSlice(sc, vals, vals2) { record1 := &RecordData{Handle: h, Values: vals2} record2 := &RecordData{Handle: h, Values: vals} return false, ErrDataInConsistent.GenWithStack("data:%#v != record:%#v", record1, record2) From 7cfd2a2d3072c30f0fc69022343658f54ed0c615 Mon Sep 17 00:00:00 2001 From: crazycs Date: Mon, 15 Oct 2018 15:00:44 +0800 Subject: [PATCH 3/3] address comment --- util/admin/admin.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/admin/admin.go b/util/admin/admin.go index b536c13f95493..325d09df98fc4 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -352,7 +352,7 @@ func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table if err != nil { return errors.Trace(err) } - sc := new(stmtctx.StatementContext) + sc := sessCtx.GetSessionVars().StmtCtx for { vals1, h, err := it.Next() if terror.ErrorEqual(err, io.EOF) { @@ -516,7 +516,7 @@ func CompareTableRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table. } startKey := t.RecordKey(0) - sc := new(stmtctx.StatementContext) + sc := sessCtx.GetSessionVars().StmtCtx filterFunc := func(h int64, vals []types.Datum, cols []*table.Column) (bool, error) { vals2, ok := m[h] if !ok {