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

planner/core: fix index out of bound bug when empty dual table is remove for mpp query (#28251) #28279

Merged
merged 2 commits into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions executor/tiflash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,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")
Expand Down
19 changes: 17 additions & 2 deletions planner/core/stringer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand All @@ -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))
}

Expand Down