*: fix bug that UnionScan can't keep order caused wrong result (pingc…

…ap#33218)

close pingcap#33175

executor/table_reader.go

@@ -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

planner/core/handle_cols.go

@@ -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.

planner/core/integration_test.go

@@ -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"))
+}