From e071841317c5977b8f0bef1237b013bce9ef63d0 Mon Sep 17 00:00:00 2001 From: Song Gao Date: Thu, 4 Aug 2022 15:50:06 +0800 Subject: [PATCH] planner: fix redudent planID for partition table in optimizer trace (#36711) close pingcap/tidb#36759 --- planner/core/logical_plan_trace_test.go | 12 ++++++------ planner/core/rule_partition_processor.go | 25 ++++++++++++++++-------- sessionctx/variable/session.go | 6 ++++++ 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/planner/core/logical_plan_trace_test.go b/planner/core/logical_plan_trace_test.go index 97e4db8ba2..5bf38b6e18 100644 --- a/planner/core/logical_plan_trace_test.go +++ b/planner/core/logical_plan_trace_test.go @@ -191,7 +191,7 @@ func TestSingleRuleTraceStep(t *testing.T) { assertRuleSteps: []assertTraceStep{ { assertReason: "DataSource_1 has multiple needed partitions[p1,p2] after pruning", - assertAction: "DataSource_1 becomes PartitionUnion_6 with children[TableScan_1,TableScan_1]", + assertAction: "DataSource_1 becomes PartitionUnion_6 with children[TableScan_7,TableScan_8]", }, }, }, @@ -202,7 +202,7 @@ func TestSingleRuleTraceStep(t *testing.T) { assertRuleSteps: []assertTraceStep{ { assertReason: "DataSource_1 has one needed partition[p1] after pruning", - assertAction: "DataSource_1 becomes TableScan_1", + assertAction: "DataSource_1 becomes TableScan_5", }, }, }, @@ -213,7 +213,7 @@ func TestSingleRuleTraceStep(t *testing.T) { assertRuleSteps: []assertTraceStep{ { assertReason: "DataSource_1 has multiple needed partitions[p1,p2] after pruning", - assertAction: "DataSource_1 becomes PartitionUnion_7 with children[TableScan_1,TableScan_1]", + assertAction: "DataSource_1 becomes PartitionUnion_7 with children[TableScan_8,TableScan_9]", }, }, }, @@ -224,7 +224,7 @@ func TestSingleRuleTraceStep(t *testing.T) { assertRuleSteps: []assertTraceStep{ { assertReason: "DataSource_1 has one needed partition[p2] after pruning", - assertAction: "DataSource_1 becomes TableScan_1", + assertAction: "DataSource_1 becomes TableScan_6", }, }, }, @@ -246,7 +246,7 @@ func TestSingleRuleTraceStep(t *testing.T) { assertRuleSteps: []assertTraceStep{ { assertReason: "DataSource_1 has multiple needed partitions[p1,p2] after pruning", - assertAction: "DataSource_1 becomes PartitionUnion_7 with children[TableScan_1,TableScan_1]", + assertAction: "DataSource_1 becomes PartitionUnion_7 with children[TableScan_8,TableScan_9]", }, }, }, @@ -257,7 +257,7 @@ func TestSingleRuleTraceStep(t *testing.T) { assertRuleSteps: []assertTraceStep{ { assertReason: "DataSource_1 has one needed partition[p1] after pruning", - assertAction: "DataSource_1 becomes TableScan_1", + assertAction: "DataSource_1 becomes TableScan_6", }, }, }, diff --git a/planner/core/rule_partition_processor.go b/planner/core/rule_partition_processor.go index b3dd100c11..6b48f56020 100644 --- a/planner/core/rule_partition_processor.go +++ b/planner/core/rule_partition_processor.go @@ -1611,9 +1611,12 @@ func (p *rangeColumnsPruner) pruneUseBinarySearch(sctx sessionctx.Context, op st return start, end } -func appendMakeUnionAllChildrenTranceStep(ds *DataSource, usedMap map[int64]model.PartitionDefinition, plan LogicalPlan, children []LogicalPlan, opt *logicalOptimizeOp) { +func appendMakeUnionAllChildrenTranceStep(origin *DataSource, usedMap map[int64]model.PartitionDefinition, plan LogicalPlan, children []LogicalPlan, opt *logicalOptimizeOp) { + if opt.tracer == nil { + return + } if len(children) == 0 { - appendNoPartitionChildTraceStep(ds, plan, opt) + appendNoPartitionChildTraceStep(origin, plan, opt) return } var action, reason func() string @@ -1625,26 +1628,32 @@ func appendMakeUnionAllChildrenTranceStep(ds *DataSource, usedMap map[int64]mode return i.ID < j.ID }) if len(children) == 1 { + newDS := plan.(*DataSource) + newDS.id = origin.SCtx().GetSessionVars().AllocNewPlanID() action = func() string { - return fmt.Sprintf("%v_%v becomes %s_%v", ds.TP(), ds.ID(), plan.TP(), plan.ID()) + return fmt.Sprintf("%v_%v becomes %s_%v", origin.TP(), origin.ID(), newDS.TP(), newDS.ID()) } reason = func() string { - return fmt.Sprintf("%v_%v has one needed partition[%s] after pruning", ds.TP(), ds.ID(), used[0].Name) + return fmt.Sprintf("%v_%v has one needed partition[%s] after pruning", origin.TP(), origin.ID(), used[0].Name) } } else { action = func() string { - buffer := bytes.NewBufferString(fmt.Sprintf("%v_%v becomes %s_%v with children[", ds.TP(), ds.ID(), plan.TP(), plan.ID())) + buffer := bytes.NewBufferString(fmt.Sprintf("%v_%v becomes %s_%v with children[", + origin.TP(), origin.ID(), plan.TP(), plan.ID())) for i, child := range children { if i > 0 { buffer.WriteString(",") } - buffer.WriteString(fmt.Sprintf("%s_%v", child.TP(), child.ID())) + newDS := child.(*DataSource) + newDS.id = origin.SCtx().GetSessionVars().AllocNewPlanID() + buffer.WriteString(fmt.Sprintf("%s_%v", child.TP(), newDS.ID())) } buffer.WriteString("]") return buffer.String() } reason = func() string { - buffer := bytes.NewBufferString(fmt.Sprintf("%v_%v has multiple needed partitions[", ds.TP(), ds.ID())) + buffer := bytes.NewBufferString(fmt.Sprintf("%v_%v has multiple needed partitions[", + origin.TP(), origin.ID())) for i, u := range used { if i > 0 { buffer.WriteString(",") @@ -1655,7 +1664,7 @@ func appendMakeUnionAllChildrenTranceStep(ds *DataSource, usedMap map[int64]mode return buffer.String() } } - opt.appendStepToCurrent(ds.ID(), ds.TP(), reason, action) + opt.appendStepToCurrent(origin.ID(), origin.TP(), reason, action) } func appendNoPartitionChildTraceStep(ds *DataSource, dual LogicalPlan, opt *logicalOptimizeOp) { diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 8a29ac4aa2..921c7a0701 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -1257,6 +1257,12 @@ func (s *SessionVars) BuildParserConfig() parser.ParserConfig { } } +// AllocNewPlanID alloc new ID +func (s *SessionVars) AllocNewPlanID() int { + s.PlanID++ + return s.PlanID +} + const ( // PlacementModeStrict indicates all placement operations should be checked strictly in ddl PlacementModeStrict string = "STRICT"