From 79f9e12152dd14d89f4090d11456df9aeb35d294 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 18 Sep 2020 20:39:30 +0800 Subject: [PATCH 1/3] executor: fix sort result for batch-point-get by unsigned pk --- executor/batch_point_get.go | 10 ++++++++-- executor/batch_point_get_test.go | 11 +++++++++++ kv/key.go | 19 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/executor/batch_point_get.go b/executor/batch_point_get.go index e41dadbb28cb0..44f593b46b62a 100644 --- a/executor/batch_point_get.go +++ b/executor/batch_point_get.go @@ -20,6 +20,7 @@ import ( "github.com/pingcap/failpoint" "github.com/pingcap/parser/model" + "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" @@ -251,11 +252,16 @@ func (e *BatchPointGetExec) initialize(ctx context.Context) error { failpoint.InjectContext(ctx, "batchPointGetRepeatableReadTest-step2", nil) }) } else if e.keepOrder { + unsignedPK := e.tblInfo.PKIsHandle && mysql.HasUnsignedFlag(e.tblInfo.GetPkColInfo().Flag) sort.Slice(e.handles, func(i int, j int) bool { + compare := e.handles[i].Compare + if unsignedPK { + compare = kv.UintHandleComparator(e.handles[i].(kv.IntHandle)).Compare + } if e.desc { - return e.handles[i].Compare(e.handles[j]) > 0 + return compare(e.handles[j]) > 0 } - return e.handles[i].Compare(e.handles[j]) < 0 + return compare(e.handles[j]) < 0 }) } diff --git a/executor/batch_point_get_test.go b/executor/batch_point_get_test.go index fc2d30538db2f..2ad4412b9869d 100644 --- a/executor/batch_point_get_test.go +++ b/executor/batch_point_get_test.go @@ -150,3 +150,14 @@ func (s *testBatchPointGetSuite) TestIssue18843(c *C) { tk.MustQuery("select * from t18843 where f in (null)").Check(testkit.Rows()) tk.MustQuery("select * from t18843 where f is null").Check(testkit.Rows("2 ")) } + +func (s *testBatchPointGetSuite) TestBatchPointGetUnsignedHandleWithSort(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("create table t (id bigint(20) unsigned, primary key(id))") + tk.MustExec("insert into t values (8738875760185212610)") + tk.MustExec("insert into t values (9814441339970117597)") + tk.MustExec("insert into t values (1)") + tk.MustQuery("select id from t where id in (8738875760185212610, 1, 9814441339970117597) order by id").Check(testkit.Rows("1", "8738875760185212610", "9814441339970117597")) + tk.MustQuery("select id from t where id in (8738875760185212610, 1, 9814441339970117597) order by id desc").Check(testkit.Rows("9814441339970117597", "8738875760185212610", "1")) +} diff --git a/kv/key.go b/kv/key.go index c5397993a0566..06ca065c1f0db 100644 --- a/kv/key.go +++ b/kv/key.go @@ -222,6 +222,25 @@ func (ih IntHandle) String() string { return strconv.FormatInt(int64(ih), 10) } +// UintHandleComparator helps compare two intHandle with unsigned int value. +type UintHandleComparator IntHandle + +// Compare returns the comparison result of the two uint handles, it panics if the types are different. +func (uh UintHandleComparator) Compare(h Handle) int { + if !h.IsInt() { + panic("IntHandle compares to CommonHandle") + } + ihVal := uint64(IntHandle(uh).IntValue()) + hVal := uint64(h.IntValue()) + if ihVal > hVal { + return 1 + } + if ihVal < hVal { + return -1 + } + return 0 +} + // CommonHandle implements the Handle interface for non-int64 type handle. type CommonHandle struct { encoded []byte From 0759c108e8d85b585dd6ff1fa586ec574d119e75 Mon Sep 17 00:00:00 2001 From: lysu Date: Fri, 18 Sep 2020 20:52:41 +0800 Subject: [PATCH 2/3] fix ci test --- executor/batch_point_get_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/executor/batch_point_get_test.go b/executor/batch_point_get_test.go index 2ad4412b9869d..468ba2e974cda 100644 --- a/executor/batch_point_get_test.go +++ b/executor/batch_point_get_test.go @@ -154,10 +154,11 @@ func (s *testBatchPointGetSuite) TestIssue18843(c *C) { func (s *testBatchPointGetSuite) TestBatchPointGetUnsignedHandleWithSort(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") - tk.MustExec("create table t (id bigint(20) unsigned, primary key(id))") - tk.MustExec("insert into t values (8738875760185212610)") - tk.MustExec("insert into t values (9814441339970117597)") - tk.MustExec("insert into t values (1)") - tk.MustQuery("select id from t where id in (8738875760185212610, 1, 9814441339970117597) order by id").Check(testkit.Rows("1", "8738875760185212610", "9814441339970117597")) - tk.MustQuery("select id from t where id in (8738875760185212610, 1, 9814441339970117597) order by id desc").Check(testkit.Rows("9814441339970117597", "8738875760185212610", "1")) + tk.MustExec("drop table if exists t2") + tk.MustExec("create table t2 (id bigint(20) unsigned, primary key(id))") + tk.MustExec("insert into t2 values (8738875760185212610)") + tk.MustExec("insert into t2 values (9814441339970117597)") + tk.MustExec("insert into t2 values (1)") + tk.MustQuery("select id from t2 where id in (8738875760185212610, 1, 9814441339970117597) order by id").Check(testkit.Rows("1", "8738875760185212610", "9814441339970117597")) + tk.MustQuery("select id from t2 where id in (8738875760185212610, 1, 9814441339970117597) order by id desc").Check(testkit.Rows("9814441339970117597", "8738875760185212610", "1")) } From b716f91dc7c284653582de0e3e9280fc51633d34 Mon Sep 17 00:00:00 2001 From: lysu Date: Wed, 23 Sep 2020 11:23:00 +0800 Subject: [PATCH 3/3] address comments --- executor/batch_point_get.go | 38 ++++++++++++++++++++++++++++--------- kv/key.go | 19 ------------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/executor/batch_point_get.go b/executor/batch_point_get.go index 44f593b46b62a..c4fdca29b4bb3 100644 --- a/executor/batch_point_get.go +++ b/executor/batch_point_get.go @@ -15,6 +15,7 @@ package executor import ( "context" + "fmt" "sort" "sync/atomic" @@ -252,17 +253,36 @@ func (e *BatchPointGetExec) initialize(ctx context.Context) error { failpoint.InjectContext(ctx, "batchPointGetRepeatableReadTest-step2", nil) }) } else if e.keepOrder { - unsignedPK := e.tblInfo.PKIsHandle && mysql.HasUnsignedFlag(e.tblInfo.GetPkColInfo().Flag) - sort.Slice(e.handles, func(i int, j int) bool { - compare := e.handles[i].Compare - if unsignedPK { - compare = kv.UintHandleComparator(e.handles[i].(kv.IntHandle)).Compare - } + less := func(i int, j int) bool { if e.desc { - return compare(e.handles[j]) > 0 + return e.handles[i].Compare(e.handles[j]) > 0 } - return compare(e.handles[j]) < 0 - }) + return e.handles[i].Compare(e.handles[j]) < 0 + + } + if e.tblInfo.PKIsHandle && mysql.HasUnsignedFlag(e.tblInfo.GetPkColInfo().Flag) { + uintComparator := func(i, h kv.Handle) int { + if !i.IsInt() || !h.IsInt() { + panic(fmt.Sprintf("both handles need be IntHandle, but got %T and %T ", i, h)) + } + ihVal := uint64(i.IntValue()) + hVal := uint64(h.IntValue()) + if ihVal > hVal { + return 1 + } + if ihVal < hVal { + return -1 + } + return 0 + } + less = func(i int, j int) bool { + if e.desc { + return uintComparator(e.handles[i], e.handles[j]) > 0 + } + return uintComparator(e.handles[i], e.handles[j]) < 0 + } + } + sort.Slice(e.handles, less) } keys := make([]kv.Key, len(e.handles)) diff --git a/kv/key.go b/kv/key.go index 06ca065c1f0db..c5397993a0566 100644 --- a/kv/key.go +++ b/kv/key.go @@ -222,25 +222,6 @@ func (ih IntHandle) String() string { return strconv.FormatInt(int64(ih), 10) } -// UintHandleComparator helps compare two intHandle with unsigned int value. -type UintHandleComparator IntHandle - -// Compare returns the comparison result of the two uint handles, it panics if the types are different. -func (uh UintHandleComparator) Compare(h Handle) int { - if !h.IsInt() { - panic("IntHandle compares to CommonHandle") - } - ihVal := uint64(IntHandle(uh).IntValue()) - hVal := uint64(h.IntValue()) - if ihVal > hVal { - return 1 - } - if ihVal < hVal { - return -1 - } - return 0 -} - // CommonHandle implements the Handle interface for non-int64 type handle. type CommonHandle struct { encoded []byte