Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

admin: fix admin check table compare bug (#7818) #7975

Merged
merged 4 commits into from
Oct 22, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions executor/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,14 @@ func (s *testSuite) TestAdminCheckTable(c *C) {
tk.MustExec(`ALTER TABLE t1 ADD INDEX idx5 (c5)`)
tk.MustExec(`ALTER TABLE t1 ADD INDEX idx6 (c6)`)
tk.MustExec(`admin check table t1`)

// Test add index on decimal column.
tk.MustExec(`drop table if exists td1;`)
tk.MustExec(`CREATE TABLE td1 (c2 INT NULL DEFAULT '70');`)
tk.MustExec(`INSERT INTO td1 SET c2 = '5';`)
tk.MustExec(`ALTER TABLE td1 ADD COLUMN c4 DECIMAL(12,8) NULL DEFAULT '213.41598062';`)
tk.MustExec(`ALTER TABLE td1 ADD INDEX id2 (c4) ;`)
tk.MustExec(`ADMIN CHECK TABLE td1;`)
}

func (s *testSuite) TestAdminCheckPrimaryIndex(c *C) {
Expand Down
1 change: 0 additions & 1 deletion executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ func (s *testSuite) TestAdmin(c *C) {
tk.MustExec("ALTER TABLE t1 ADD COLUMN c4 bit(10) default 127;")
tk.MustExec("ALTER TABLE t1 ADD INDEX idx3 (c4);")
tk.MustExec("admin check table t1;")

}

func (s *testSuite) fillData(tk *testkit.TestKit, table string) {
Expand Down
38 changes: 38 additions & 0 deletions types/datum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package types

import (
"reflect"
"testing"
"time"

. "github.com/pingcap/check"
Expand Down Expand Up @@ -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)
}
}
20 changes: 17 additions & 3 deletions util/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package admin
import (
"fmt"
"io"
"reflect"
"sort"

"github.com/pingcap/tidb/expression"
Expand Down Expand Up @@ -353,6 +352,7 @@ func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table
if err != nil {
return errors.Trace(err)
}
sc := sessCtx.GetSessionVars().StmtCtx
for {
vals1, h, err := it.Next()
if terror.ErrorEqual(err, io.EOF) {
Expand All @@ -375,7 +375,7 @@ func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table
return errors.Trace(err)
}
adjustDatumKind(vals1, vals2)
if !reflect.DeepEqual(vals1, vals2) {
if !compareDatumSlice(sc, vals1, vals2) {
record1 := &RecordData{Handle: h, Values: vals1}
record2 := &RecordData{Handle: h, Values: vals2}
return ErrDataInConsistent.GenWithStack("index:%#v != record:%#v", record1, record2)
Expand All @@ -385,6 +385,19 @@ func checkIndexAndRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table
return nil
}

func compareDatumSlice(sc *stmtctx.StatementContext, val1s, val2s []types.Datum) bool {
if len(val1s) != len(val2s) {
return false
}
for i, v := range val1s {
res, err := v.CompareDatum(sc, &val2s[i])
if err != nil || res != 0 {
return false
}
}
return true
}

// CheckRecordAndIndex is exported for testing.
func CheckRecordAndIndex(sessCtx sessionctx.Context, txn kv.Transaction, t table.Table, idx table.Index, genExprs map[string]expression.Expression) error {
sc := sessCtx.GetSessionVars().StmtCtx
Expand Down Expand Up @@ -503,6 +516,7 @@ func CompareTableRecord(sessCtx sessionctx.Context, txn kv.Transaction, t table.
}

startKey := t.RecordKey(0)
sc := sessCtx.GetSessionVars().StmtCtx
filterFunc := func(h int64, vals []types.Datum, cols []*table.Column) (bool, error) {
vals2, ok := m[h]
if !ok {
Expand All @@ -514,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)
Expand Down