Skip to content

Commit

Permalink
*: fix bug that UnionScan can't keep order caused wrong result (#33218)…
Browse files Browse the repository at this point in the history
… (#37805)

close #33175
  • Loading branch information
ti-srebot authored Sep 22, 2022
1 parent 6f15862 commit e617b92
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 9 deletions.
12 changes: 12 additions & 0 deletions executor/table_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,18 @@ func (e *TableReaderExecutor) Open(ctx context.Context) error {
// Calculate the kv ranges here, UnionScan rely on this kv ranges.
// cached table and temporary table are similar
if e.table.Meta() != nil && e.table.Meta().TempTableType != model.TempTableNone {
if e.desc && len(secondPartRanges) != 0 {
// TiKV support reverse scan and the `resultHandler` process the range order.
// While in UnionScan, it doesn't use reverse scan and reverse the final result rows manually.
// So things are differ, we need to reverse the kv range here.
// TODO: If we refactor UnionScan to use reverse scan, update the code here.
// [9734095886065816708 9734095886065816709] | [1 3] [65535 9734095886065816707] => before the following change
// [1 3] [65535 9734095886065816707] | [9734095886065816708 9734095886065816709] => ranges part reverse here
// [1 3 65535 9734095886065816707 9734095886065816708 9734095886065816709] => scan (normal order) in UnionScan
// [9734095886065816709 9734095886065816708 9734095886065816707 65535 3 1] => rows reverse in UnionScan
firstPartRanges, secondPartRanges = secondPartRanges, firstPartRanges
}

kvReq, err := e.buildKVReq(ctx, firstPartRanges)
if err != nil {
return err
Expand Down
12 changes: 3 additions & 9 deletions planner/core/handle_cols.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,15 +239,9 @@ func (ib *IntHandleCols) NumCols() int {

// Compare implements the kv.HandleCols interface.
func (ib *IntHandleCols) Compare(a, b []types.Datum, ctors []collate.Collator) (int, error) {
aInt := a[ib.col.Index].GetInt64()
bInt := b[ib.col.Index].GetInt64()
if aInt == bInt {
return 0, nil
}
if aInt < bInt {
return -1, nil
}
return 1, nil
aVal := &a[ib.col.Index]
bVal := &b[ib.col.Index]
return aVal.Compare(nil, bVal, ctors[ib.col.Index])
}

// GetFieldsTypes implements the kv.HandleCols interface.
Expand Down
56 changes: 56 additions & 0 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5347,3 +5347,59 @@ func (s *testIntegrationSuite) TestDNFCondSelectivityWithConst(c *C) {
"[ └─TableFullScan_5 129.00 cop[tikv] table:t1 keep order:false"))
testKit.MustExec("drop table if exists t1")
}

func (s *testIntegrationSuite) TestIssue33175(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("create table t (id bigint(45) unsigned not null, c varchar(20), primary key(id));")
tk.MustExec("insert into t values (9734095886065816707, 'a'), (10353107668348738101, 'b'), (0, 'c');")
tk.MustExec("begin")
tk.MustExec("insert into t values (33, 'd');")
tk.MustQuery("select max(id) from t;").Check(testkit.Rows("10353107668348738101"))
tk.MustExec("rollback")

// With subquery, like the original issue case.
tk.MustQuery("select * from t where id > (select max(id) from t where t.id > 0);").Check(testkit.Rows())

// Test order by desc / asc.
tk.MustQuery("select id from t order by id desc;").Check(testkit.Rows(
"10353107668348738101",
"9734095886065816707",
"0"))

tk.MustQuery("select id from t order by id asc;").Check(testkit.Rows(
"0",
"9734095886065816707",
"10353107668348738101"))

tk.MustExec("drop table t")

// Cover more code that use union scan
// TableReader/IndexReader/IndexLookup
for idx, q := range []string{
"create temporary table t (id bigint unsigned, c int default null, index(id))",
"create temporary table t (id bigint unsigned primary key)",
} {
tk.MustExec(q)
tk.MustExec("insert into t(id) values (1), (3), (9734095886065816707), (9734095886065816708)")
tk.MustQuery("select min(id) from t").Check(testkit.Rows("1"))
tk.MustQuery("select max(id) from t").Check(testkit.Rows("9734095886065816708"))
tk.MustQuery("select id from t order by id asc").Check(testkit.Rows(
"1", "3", "9734095886065816707", "9734095886065816708"))
tk.MustQuery("select id from t order by id desc").Check(testkit.Rows(
"9734095886065816708", "9734095886065816707", "3", "1"))
if idx == 0 {
tk.MustQuery("select * from t order by id asc").Check(testkit.Rows(
"1 <nil>",
"3 <nil>",
"9734095886065816707 <nil>",
"9734095886065816708 <nil>"))
tk.MustQuery("select * from t order by id desc").Check(testkit.Rows(
"9734095886065816708 <nil>",
"9734095886065816707 <nil>",
"3 <nil>",
"1 <nil>"))
}
tk.MustExec("drop table t")
}
}

0 comments on commit e617b92

Please sign in to comment.