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
… (#33319)

close #33175
  • Loading branch information
ti-srebot authored Mar 24, 2022
1 parent 3fa3e0e commit 4938619
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 9 deletions.
11 changes: 11 additions & 0 deletions executor/table_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,17 @@ 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.dummy {
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
89 changes: 89 additions & 0 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6243,3 +6243,92 @@ func TestTiFlashPartitionTableScan(t *testing.T) {
tk.MustExec("drop table rp_t;")
tk.MustExec("drop table hp_t;")
}

func TestIssue33175(t *testing.T) {
store, _, clean := testkit.CreateMockStoreAndDomain(t)
defer clean()
tk := testkit.NewTestKit(t, 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")

tk.MustExec("alter table t cache")
for {
tk.MustQuery("select max(id) from t;").Check(testkit.Rows("10353107668348738101"))
if tk.Session().GetSessionVars().StmtCtx.ReadFromTableCache {
break
}
}

// // With subquery, like the original issue case.
for {
tk.MustQuery("select * from t where id > (select max(id) from t where t.id > 0);").Check(testkit.Rows())
if tk.Session().GetSessionVars().StmtCtx.ReadFromTableCache {
break
}
}

// 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("alter table t nocache")
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")
}

// More and more test
tk.MustExec("create global temporary table `tmp1` (id bigint unsigned primary key) on commit delete rows;")
tk.MustExec("begin")
tk.MustExec("insert into tmp1 values (0),(1),(2),(65536),(9734095886065816707),(9734095886065816708);")
tk.MustQuery("select * from tmp1 where id <= 65534 or (id > 65535 and id < 9734095886065816700) or id >= 9734095886065816707 order by id desc;").Check(testkit.Rows(
"9734095886065816708", "9734095886065816707", "65536", "2", "1", "0"))

tk.MustQuery("select * from tmp1 where id <= 65534 or (id > 65535 and id < 9734095886065816700) or id >= 9734095886065816707 order by id asc;").Check(testkit.Rows(
"0", "1", "2", "65536", "9734095886065816707", "9734095886065816708"))

tk.MustExec("create global temporary table `tmp2` (id bigint primary key) on commit delete rows;")
tk.MustExec("begin")
tk.MustExec("insert into tmp2 values(-2),(-1),(0),(1),(2);")
tk.MustQuery("select * from tmp2 where id <= -1 or id > 0 order by id desc;").Check(testkit.Rows("2", "1", "-1", "-2"))
tk.MustQuery("select * from tmp2 where id <= -1 or id > 0 order by id asc;").Check(testkit.Rows("-2", "-1", "1", "2"))
}

0 comments on commit 4938619

Please sign in to comment.