From fa43b3d620eb3b5a262be390cfe62346dc00b284 Mon Sep 17 00:00:00 2001 From: Zhi Qi Date: Wed, 18 May 2022 12:25:37 +0800 Subject: [PATCH 1/4] fix --- planner/core/task.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/planner/core/task.go b/planner/core/task.go index 6684d26b80534..ddd6f24b29b51 100644 --- a/planner/core/task.go +++ b/planner/core/task.go @@ -1891,12 +1891,21 @@ func (p *PhysicalHashAgg) attach2Task(tasks ...task) task { return t } +func (p *PhysicalWindow) attach2TaskForMPP(mpp *mppTask) task { + // FIXME: currently, tiflash's join has different schema with TiDB, + // so we have to rebuild the schema of join and operators which may inherit schema from join. + // for window, we take the sub-plan's schema, and the schema generated by windowDescs. + columns := p.Schema().Clone().Columns[len(p.Schema().Columns)-len(p.WindowFuncDescs):] + p.schema = expression.MergeSchema(mpp.plan().Schema(), expression.NewSchema(columns...)) + + // TODO: find a better way to solve the cost problem. + mpp.cst = mpp.cost() * 0.05 + p.cost = mpp.cost() + return attachPlan2Task(p, mpp) +} func (p *PhysicalWindow) attach2Task(tasks ...task) task { if mpp, ok := tasks[0].copy().(*mppTask); ok && p.storeTp == kv.TiFlash { - // TODO: find a better way to solve the cost problem. - mpp.cst = mpp.cost() * 0.05 - p.cost = mpp.cost() - return attachPlan2Task(p, mpp) + return p.attach2TaskForMPP(mpp) } t := tasks[0].convertToRootTask(p.ctx) p.cost = t.cost() From 941b189d3ab21c5e1c05cef6215b4a7be3bdf52a Mon Sep 17 00:00:00 2001 From: Zhi Qi Date: Wed, 18 May 2022 13:21:07 +0800 Subject: [PATCH 2/4] add failpoint --- planner/core/task.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/planner/core/task.go b/planner/core/task.go index ddd6f24b29b51..3f9c117f04392 100644 --- a/planner/core/task.go +++ b/planner/core/task.go @@ -18,6 +18,7 @@ import ( "math" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/expression/aggregation" "github.com/pingcap/tidb/kv" @@ -1897,12 +1898,18 @@ func (p *PhysicalWindow) attach2TaskForMPP(mpp *mppTask) task { // for window, we take the sub-plan's schema, and the schema generated by windowDescs. columns := p.Schema().Clone().Columns[len(p.Schema().Columns)-len(p.WindowFuncDescs):] p.schema = expression.MergeSchema(mpp.plan().Schema(), expression.NewSchema(columns...)) + failpoint.Inject("CheckWindowSchemaLength", func() { + if len(p.Schema().Columns) <= len(mpp.plan().Schema().Columns) { + panic("mpp physical window's schema should be longer than sub operator's schema.") + } + }) // TODO: find a better way to solve the cost problem. mpp.cst = mpp.cost() * 0.05 p.cost = mpp.cost() return attachPlan2Task(p, mpp) } + func (p *PhysicalWindow) attach2Task(tasks ...task) task { if mpp, ok := tasks[0].copy().(*mppTask); ok && p.storeTp == kv.TiFlash { return p.attach2TaskForMPP(mpp) From a1250bcdb54dbd74a9c96cad7f3b41fcd07bc5ea Mon Sep 17 00:00:00 2001 From: Zhi Qi Date: Wed, 18 May 2022 13:59:50 +0800 Subject: [PATCH 3/4] add test --- planner/core/task.go | 7 ++++--- planner/core/window_push_down_test.go | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/planner/core/task.go b/planner/core/task.go index 3f9c117f04392..a60cbf2b7ebf0 100644 --- a/planner/core/task.go +++ b/planner/core/task.go @@ -1898,9 +1898,10 @@ func (p *PhysicalWindow) attach2TaskForMPP(mpp *mppTask) task { // for window, we take the sub-plan's schema, and the schema generated by windowDescs. columns := p.Schema().Clone().Columns[len(p.Schema().Columns)-len(p.WindowFuncDescs):] p.schema = expression.MergeSchema(mpp.plan().Schema(), expression.NewSchema(columns...)) - failpoint.Inject("CheckWindowSchemaLength", func() { - if len(p.Schema().Columns) <= len(mpp.plan().Schema().Columns) { - panic("mpp physical window's schema should be longer than sub operator's schema.") + + failpoint.Inject("CheckMPPWindowSchemaLength", func() { + if len(p.Schema().Columns) != len(mpp.plan().Schema().Columns)+len(p.WindowFuncDescs) { + panic("mpp physical window has incorrect schema length") } }) diff --git a/planner/core/window_push_down_test.go b/planner/core/window_push_down_test.go index 382f009a3f4ff..d2ce2c3c60d95 100644 --- a/planner/core/window_push_down_test.go +++ b/planner/core/window_push_down_test.go @@ -15,6 +15,7 @@ package core_test import ( + "github.com/pingcap/failpoint" "strings" "testing" @@ -122,3 +123,23 @@ func TestWindowPlanWithOtherOperators(t *testing.T) { suiteData.GetTestCases(t, &input, &output) testWithData(t, tk, input, output) } + +func TestIssue34765(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() + tk := testkit.NewTestKit(t, store) + dom := domain.GetDomain(tk.Session()) + + tk.MustExec("use test") + tk.MustExec("create table t1(c1 varchar(32), c2 datetime, c3 bigint, c4 varchar(64));") + tk.MustExec("create table t2(b2 varchar(64));") + tk.MustExec("set tidb_enforce_mpp=1;") + SetTiFlashReplica(t, dom, "test", "t1") + SetTiFlashReplica(t, dom, "test", "t2") + + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/planner/core/CheckMPPWindowSchemaLength", "return")) + defer func() { + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/planner/core/CheckMPPWindowSchemaLength")) + }() + tk.MustExec("explain select count(*) from (select row_number() over (partition by c1 order by c2) num from (select * from t1 left join t2 on t1.c4 = t2.b2) tem2 ) tx where num = 1;") +} From eddabbb585224ea238c67e9ec305080569ba77ad Mon Sep 17 00:00:00 2001 From: Zhi Qi Date: Wed, 18 May 2022 14:01:03 +0800 Subject: [PATCH 4/4] sort import --- planner/core/window_push_down_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/window_push_down_test.go b/planner/core/window_push_down_test.go index d2ce2c3c60d95..a8ec9c0955d56 100644 --- a/planner/core/window_push_down_test.go +++ b/planner/core/window_push_down_test.go @@ -15,10 +15,10 @@ package core_test import ( - "github.com/pingcap/failpoint" "strings" "testing" + "github.com/pingcap/failpoint" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/parser/model" plannercore "github.com/pingcap/tidb/planner/core"