Skip to content

Commit

Permalink
executor: fix the wrong order by pk desc result and some corner cas…
Browse files Browse the repository at this point in the history
…es for unsigned pk (#10179) (#10209)
  • Loading branch information
winoros authored and zz-jason committed Apr 22, 2019
1 parent 693c0a5 commit 377c00d
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 17 deletions.
2 changes: 1 addition & 1 deletion executor/analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 21 additions & 11 deletions executor/distsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}

Expand Down
12 changes: 12 additions & 0 deletions executor/distsql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,15 @@ func (s *testSuite) TestUniqueKeyNullValueSelect(c *C) {
res = tk.MustQuery("select * from t where id is null;")
res.Check(testkit.Rows("<nil> a", "<nil> b", "<nil> 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"))
}
12 changes: 7 additions & 5 deletions executor/table_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 377c00d

Please sign in to comment.