From a1f2eb124018ac7ec7d4564d15e4779cd5adfc6a Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Tue, 7 Feb 2023 14:41:02 +0800 Subject: [PATCH 1/3] fix can't find proper physical plan caused by virtual column Signed-off-by: AilinKid <314806019@qq.com> --- executor/tiflashtest/tiflash_test.go | 27 ++++++++++++++++++++++++++ planner/core/exhaust_physical_plans.go | 17 ++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/executor/tiflashtest/tiflash_test.go b/executor/tiflashtest/tiflash_test.go index e8cd94d889188..1af01591b8c38 100644 --- a/executor/tiflashtest/tiflash_test.go +++ b/executor/tiflashtest/tiflash_test.go @@ -1233,6 +1233,33 @@ func TestGroupStreamAggOnTiFlash(t *testing.T) { } } +// TestIssue41014 test issue that can't find proper physical plan +func TestIssue41014(t *testing.T) { + store := testkit.CreateMockStore(t, withMockTiFlash(2)) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test;") + tk.MustExec("CREATE TABLE `tai1` (\n `aid` int(11) DEFAULT NULL,\n `rid` int(11) DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin") + tk.MustExec("CREATE TABLE `tai2` (\n `rid` int(11) DEFAULT NULL,\n `prilan` varchar(20) DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin") + tk.MustExec("alter table tai1 set tiflash replica 1") + tk.MustExec("alter table tai2 set tiflash replica 1") + tk.MustExec("alter table tai2 add index idx((lower(prilan)));") + tk.MustExec("set @@tidb_opt_distinct_agg_push_down = 1;") + + tk.MustQuery("explain select count(distinct tai1.aid) as cb from tai1 inner join tai2 on tai1.rid = tai2.rid where lower(prilan) LIKE LOWER('%python%');").Check( + testkit.Rows("HashAgg_11 1.00 root funcs:count(distinct test.tai1.aid)->Column#8", + "└─HashJoin_15 9990.00 root inner join, equal:[eq(test.tai2.rid, test.tai1.rid)]", + " ├─Selection_20(Build) 8000.00 root like(lower(test.tai2.prilan), \"%python%\", 92)", + " │ └─Projection_19 10000.00 root test.tai2.rid, lower(test.tai2.prilan)", + " │ └─TableReader_18 9990.00 root data:Selection_17", + " │ └─Selection_17 9990.00 cop[tikv] not(isnull(test.tai2.rid))", + " │ └─TableFullScan_16 10000.00 cop[tikv] table:tai2 keep order:false, stats:pseudo", + " └─TableReader_23(Probe) 9990.00 root data:Selection_22", + " └─Selection_22 9990.00 cop[tikv] not(isnull(test.tai1.rid))", + " └─TableFullScan_21 10000.00 cop[tikv] table:tai1 keep order:false, stats:pseudo")) + tk.MustQuery("select count(distinct tai1.aid) as cb from tai1 inner join tai2 on tai1.rid = tai2.rid where lower(prilan) LIKE LOWER('%python%');").Check( + testkit.Rows("0")) +} + func TestTiflashEmptyDynamicPruneResult(t *testing.T) { store := testkit.CreateMockStore(t, withMockTiFlash(2)) tk := testkit.NewTestKit(t, store) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 205a2d8242b4b..6ed57b07b6cf8 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -2698,6 +2698,10 @@ func (la *LogicalAggregation) getStreamAggs(prop *property.PhysicalProperty) []P // TODO: remove AllowDistinctAggPushDown after the cost estimation of distinct pushdown is implemented. // If AllowDistinctAggPushDown is set to true, we should not consider RootTask. if !la.ctx.GetSessionVars().AllowDistinctAggPushDown { + // if variable doesn't allow DistinctAggPushDown, just produce root task type. + taskTypes = []property.TaskType{property.RootTaskType} + } else if !la.canPushToCop(kv.TiKV) { + // if variable does allow DistinctAggPushDown, but OP itself can't be pushed down to tikv, just produce root task type. taskTypes = []property.TaskType{property.RootTaskType} } else if !la.distinctArgsMeetsProperty() { continue @@ -2849,6 +2853,15 @@ func (la *LogicalAggregation) tryToGetMppHashAggs(prop *property.PhysicalPropert return } +// getHashAggs will generate some kinds of taskType here, which finally converted to different task plan. +// when deciding whether to add a kind of taskType, there is a rule here. [Not is Not, Yes is not Sure] +// eg: which means +// +// 1: when you find something here that block hashAgg to be pushed down to XXX, just skip adding the XXXTaskType. +// 2: when you find nothing here to block hashAgg to be pushed down to XXX, just add the XXXTaskType here. +// for 2, the final result for this physical operator enumeration is chosen or rejected is according to more factors later (hint/variable/partition/virtual-col/cost) +// +// That is to say, the non-complete positive judgement of canPushDownToMPP/canPushDownToTiFlash/canPushDownToTiKV is not that for sure here. func (la *LogicalAggregation) getHashAggs(prop *property.PhysicalProperty) []PhysicalPlan { if !prop.IsSortItemEmpty() { return nil @@ -2863,6 +2876,10 @@ func (la *LogicalAggregation) getHashAggs(prop *property.PhysicalProperty) []Phy if la.HasDistinct() { // TODO: remove after the cost estimation of distinct pushdown is implemented. if !la.ctx.GetSessionVars().AllowDistinctAggPushDown { + // if variable doesn't allow DistinctAggPushDown, just produce root task type. + taskTypes = []property.TaskType{property.RootTaskType} + } else if !la.canPushToCop(kv.TiKV) { + // if variable does allow DistinctAggPushDown, but OP itself can't be pushed down to tikv, just produce root task type. taskTypes = []property.TaskType{property.RootTaskType} } } else if !la.aggHints.preferAggToCop { From 8124abe79f2884aa7e772a258255f387b1d6fc90 Mon Sep 17 00:00:00 2001 From: Arenatlx Date: Wed, 8 Feb 2023 11:53:22 +0800 Subject: [PATCH 2/3] Update planner/core/exhaust_physical_plans.go Co-authored-by: Zhou Kunqin <25057648+time-and-fate@users.noreply.github.com> --- planner/core/exhaust_physical_plans.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index d4b2576a0d81a..0ee5a325899bd 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -2878,11 +2878,12 @@ func (la *LogicalAggregation) getHashAggs(prop *property.PhysicalProperty) []Phy canPushDownToMPP := canPushDownToTiFlash && la.ctx.GetSessionVars().IsMPPAllowed() && la.checkCanPushDownToMPP() if la.HasDistinct() { // TODO: remove after the cost estimation of distinct pushdown is implemented. - if !la.ctx.GetSessionVars().AllowDistinctAggPushDown { - // if variable doesn't allow DistinctAggPushDown, just produce root task type. + // if variable doesn't allow DistinctAggPushDown, just produce root task type. + if !la.ctx.GetSessionVars().AllowDistinctAggPushDown || + // if variable does allow DistinctAggPushDown, but OP itself can't be pushed down to tikv, just produce root task type. + !la.canPushToCop(kv.TiKV) { taskTypes = []property.TaskType{property.RootTaskType} - } else if !la.canPushToCop(kv.TiKV) { - // if variable does allow DistinctAggPushDown, but OP itself can't be pushed down to tikv, just produce root task type. + taskTypes = []property.TaskType{property.RootTaskType} } } else if !la.aggHints.preferAggToCop { From e16ee0ec12d92bd991bc057994ec3b8dedbb7b1c Mon Sep 17 00:00:00 2001 From: AilinKid <314806019@qq.com> Date: Wed, 8 Feb 2023 11:57:45 +0800 Subject: [PATCH 3/3] address comment Signed-off-by: AilinKid <314806019@qq.com> --- planner/core/exhaust_physical_plans.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 0ee5a325899bd..4c8a675af5d93 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -2700,10 +2700,8 @@ func (la *LogicalAggregation) getStreamAggs(prop *property.PhysicalProperty) []P if la.HasDistinct() { // TODO: remove AllowDistinctAggPushDown after the cost estimation of distinct pushdown is implemented. // If AllowDistinctAggPushDown is set to true, we should not consider RootTask. - if !la.ctx.GetSessionVars().AllowDistinctAggPushDown { + if !la.ctx.GetSessionVars().AllowDistinctAggPushDown || !la.canPushToCop(kv.TiKV) { // if variable doesn't allow DistinctAggPushDown, just produce root task type. - taskTypes = []property.TaskType{property.RootTaskType} - } else if !la.canPushToCop(kv.TiKV) { // if variable does allow DistinctAggPushDown, but OP itself can't be pushed down to tikv, just produce root task type. taskTypes = []property.TaskType{property.RootTaskType} } else if !la.distinctArgsMeetsProperty() { @@ -2878,12 +2876,9 @@ func (la *LogicalAggregation) getHashAggs(prop *property.PhysicalProperty) []Phy canPushDownToMPP := canPushDownToTiFlash && la.ctx.GetSessionVars().IsMPPAllowed() && la.checkCanPushDownToMPP() if la.HasDistinct() { // TODO: remove after the cost estimation of distinct pushdown is implemented. - // if variable doesn't allow DistinctAggPushDown, just produce root task type. - if !la.ctx.GetSessionVars().AllowDistinctAggPushDown || - // if variable does allow DistinctAggPushDown, but OP itself can't be pushed down to tikv, just produce root task type. - !la.canPushToCop(kv.TiKV) { - taskTypes = []property.TaskType{property.RootTaskType} - + if !la.ctx.GetSessionVars().AllowDistinctAggPushDown || !la.canPushToCop(kv.TiKV) { + // if variable doesn't allow DistinctAggPushDown, just produce root task type. + // if variable does allow DistinctAggPushDown, but OP itself can't be pushed down to tikv, just produce root task type. taskTypes = []property.TaskType{property.RootTaskType} } } else if !la.aggHints.preferAggToCop {