From 377c00d7d643b6878e8601fe500a8ac3d9a163e2 Mon Sep 17 00:00:00 2001 From: Yiding Cui Date: Mon, 22 Apr 2019 19:32:27 +0800 Subject: [PATCH] executor: fix the wrong `order by pk desc` result and some corner cases for unsigned pk (#10179) (#10209) --- executor/analyze.go | 2 +- executor/distsql.go | 32 +++++++++++++++++++++----------- executor/distsql_test.go | 12 ++++++++++++ executor/table_reader.go | 12 +++++++----- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/executor/analyze.go b/executor/analyze.go index 06b59b41a7746..521e5a72181bb 100644 --- a/executor/analyze.go +++ b/executor/analyze.go @@ -324,7 +324,7 @@ func (e *AnalyzeColumnsExec) open() error { ranges = ranger.FullIntRange(false) } e.resultHandler = &tableResultHandler{} - firstPartRanges, secondPartRanges := splitRanges(ranges, e.keepOrder) + firstPartRanges, secondPartRanges := splitRanges(ranges, e.keepOrder, false) firstResult, err := e.buildResp(firstPartRanges) if err != nil { return errors.Trace(err) diff --git a/executor/distsql.go b/executor/distsql.go index bf4794fa2d511..15e6386a1ce0b 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -156,7 +156,7 @@ func handleIsExtra(col *expression.Column) bool { return false } -func splitRanges(ranges []*ranger.Range, keepOrder bool) ([]*ranger.Range, []*ranger.Range) { +func splitRanges(ranges []*ranger.Range, keepOrder bool, desc bool) ([]*ranger.Range, []*ranger.Range) { if len(ranges) == 0 || ranges[0].LowVal[0].Kind() == types.KindInt64 { return ranges, nil } @@ -170,27 +170,37 @@ func splitRanges(ranges []*ranger.Range, keepOrder bool) ([]*ranger.Range, []*ra if !keepOrder { return append(unsignedRanges, signedRanges...), nil } + if desc { + return unsignedRanges, signedRanges + } return signedRanges, unsignedRanges } signedRanges := make([]*ranger.Range, 0, idx+1) unsignedRanges := make([]*ranger.Range, 0, len(ranges)-idx) signedRanges = append(signedRanges, ranges[0:idx]...) - signedRanges = append(signedRanges, &ranger.Range{ - LowVal: ranges[idx].LowVal, - LowExclude: ranges[idx].LowExclude, - HighVal: []types.Datum{types.NewUintDatum(math.MaxInt64)}, - }) - unsignedRanges = append(unsignedRanges, &ranger.Range{ - LowVal: []types.Datum{types.NewUintDatum(math.MaxInt64 + 1)}, - HighVal: ranges[idx].HighVal, - HighExclude: ranges[idx].HighExclude, - }) + if !(ranges[idx].LowVal[0].GetUint64() == math.MaxInt64 && ranges[idx].LowExclude) { + signedRanges = append(signedRanges, &ranger.Range{ + LowVal: ranges[idx].LowVal, + LowExclude: ranges[idx].LowExclude, + HighVal: []types.Datum{types.NewUintDatum(math.MaxInt64)}, + }) + } + if !(ranges[idx].HighVal[0].GetUint64() == math.MaxInt64+1 && ranges[idx].HighExclude) { + unsignedRanges = append(unsignedRanges, &ranger.Range{ + LowVal: []types.Datum{types.NewUintDatum(math.MaxInt64 + 1)}, + HighVal: ranges[idx].HighVal, + HighExclude: ranges[idx].HighExclude, + }) + } if idx < len(ranges) { unsignedRanges = append(unsignedRanges, ranges[idx+1:]...) } if !keepOrder { return append(unsignedRanges, signedRanges...), nil } + if desc { + return unsignedRanges, signedRanges + } return signedRanges, unsignedRanges } diff --git a/executor/distsql_test.go b/executor/distsql_test.go index c4b29c80071ca..804782c591e0d 100644 --- a/executor/distsql_test.go +++ b/executor/distsql_test.go @@ -207,3 +207,15 @@ func (s *testSuite) TestUniqueKeyNullValueSelect(c *C) { res = tk.MustQuery("select * from t where id is null;") res.Check(testkit.Rows(" a", " b", " c")) } + +// TestIssue10178 contains tests for https://github.com/pingcap/tidb/issues/10178 . +func (s *testSuite) TestIssue10178(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create table t(a bigint unsigned primary key)") + tk.MustExec("insert into t values(9223372036854775807), (18446744073709551615)") + tk.MustQuery("select max(a) from t").Check(testkit.Rows("18446744073709551615")) + tk.MustQuery("select * from t where a > 9223372036854775807").Check(testkit.Rows("18446744073709551615")) + tk.MustQuery("select * from t where a < 9223372036854775808").Check(testkit.Rows("9223372036854775807")) +} diff --git a/executor/table_reader.go b/executor/table_reader.go index 9c0e528b793e1..634b9bbf7a799 100644 --- a/executor/table_reader.go +++ b/executor/table_reader.go @@ -96,7 +96,7 @@ func (e *TableReaderExecutor) Open(ctx context.Context) error { } e.resultHandler = &tableResultHandler{} - firstPartRanges, secondPartRanges := splitRanges(e.ranges, e.keepOrder) + firstPartRanges, secondPartRanges := splitRanges(e.ranges, e.keepOrder, e.desc) firstResult, err := e.buildResp(ctx, firstPartRanges) if err != nil { e.feedback.Invalidate() @@ -161,10 +161,12 @@ func (e *TableReaderExecutor) buildResp(ctx context.Context, ranges []*ranger.Ra } type tableResultHandler struct { - // If the pk is unsigned and we have KeepOrder=true. - // optionalResult handles the request whose range is in signed int range. - // result handles the request whose range is exceed signed int range. - // Otherwise, we just set optionalFinished true and the result handles the whole ranges. + // If the pk is unsigned and we have KeepOrder=true and want ascending order, + // `optionalResult` will handles the request whose range is in signed int range, and + // `result` will handle the request whose range is exceed signed int range. + // If we want descending order, `optionalResult` will handles the request whose range is exceed signed, and + // the `result` will handle the request whose range is in signed. + // Otherwise, we just set `optionalFinished` true and the `result` handles the whole ranges. optionalResult distsql.SelectResult result distsql.SelectResult