planner/core: fix index out of bound bug when empty dual table is rem…

…ove for mpp query (#28251) (#28278)
pingcap · Dec 17, 2021 · 66850a6 · 66850a6
1 parent 50c26c1
commit 66850a6
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 2 deletions.
diff --git a/executor/tiflash_test.go b/executor/tiflash_test.go
@@ -520,6 +520,28 @@ func (s *tiflashTestSuite) TestMppUnionAll(c *C) {
 
 }
 
+func (s *tiflashTestSuite) TestUnionWithEmptyDualTable(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+	tk.MustExec("use test")
+	tk.MustExec("drop table if exists t")
+	tk.MustExec("drop table if exists t1")
+	tk.MustExec("create table t (a int not null, b int, c varchar(20))")
+	tk.MustExec("create table t1 (a int, b int not null, c double)")
+	tk.MustExec("alter table t set tiflash replica 1")
+	tk.MustExec("alter table t1 set tiflash replica 1")
+	tb := testGetTableByName(c, tk.Se, "test", "t")
+	err := domain.GetDomain(tk.Se).DDL().UpdateTableReplicaInfo(tk.Se, tb.Meta().ID, true)
+	c.Assert(err, IsNil)
+	tb = testGetTableByName(c, tk.Se, "test", "t1")
+	err = domain.GetDomain(tk.Se).DDL().UpdateTableReplicaInfo(tk.Se, tb.Meta().ID, true)
+	c.Assert(err, IsNil)
+	tk.MustExec("insert into t values(1,2,3)")
+	tk.MustExec("insert into t1 values(1,2,3)")
+	tk.MustExec("set @@session.tidb_isolation_read_engines=\"tiflash\"")
+	tk.MustExec("set @@session.tidb_enforce_mpp=ON")
+	tk.MustQuery("select count(*) from (select a , b from t union all select a , c from t1 where false) tt").Check(testkit.Rows("1"))
+}
+
 func (s *tiflashTestSuite) TestMppApply(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 	tk.MustExec("use test")

diff --git a/planner/core/stringer.go b/planner/core/stringer.go
@@ -27,10 +27,25 @@ func ToString(p Plan) string {
 	return strings.Join(strs, "->")
 }
 
+func needIncludeChildrenString(plan Plan) bool {
+	switch x := plan.(type) {
+	case *LogicalUnionAll, *PhysicalUnionAll, *LogicalPartitionUnionAll:
+		// after https://github.com/pingcap/tidb/pull/25218, the union may contain less than 2 children,
+		// but we still wants to include its child plan's information when calling `toString` on union.
+		return true
+	case LogicalPlan:
+		return len(x.Children()) > 1
+	case PhysicalPlan:
+		return len(x.Children()) > 1
+	default:
+		return false
+	}
+}
+
 func toString(in Plan, strs []string, idxs []int) ([]string, []int) {
 	switch x := in.(type) {
 	case LogicalPlan:
-		if len(x.Children()) > 1 {
+		if needIncludeChildrenString(in) {
 			idxs = append(idxs, len(strs))
 		}
 
@@ -39,7 +54,7 @@ func toString(in Plan, strs []string, idxs []int) ([]string, []int) {
 		}
 	case *PhysicalExchangeReceiver: // do nothing
 	case PhysicalPlan:
-		if len(x.Children()) > 1 {
+		if needIncludeChildrenString(in) {
 			idxs = append(idxs, len(strs))
 		}