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

executor: fix the wrong order by pk desc result and some corner cases for unsigned pk (#10179) #10209

Merged
merged 4 commits into from
Apr 22, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
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