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

*: fix bug that UnionScan can't keep order caused wrong result #33218

Merged
merged 10 commits into from
Mar 22, 2022
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)
tiancaiamao marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -6239,3 +6239,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"))
}