address the comment

Signed-off-by: yisaer <disxiaofei@163.com>
pingcap · Dec 2, 2021 · 42db16e · 42db16e
1 parent c7b5b57
commit 42db16e
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 6 deletions.
diff --git a/planner/core/logical_plan_trace_test.go b/planner/core/logical_plan_trace_test.go
@@ -119,7 +119,7 @@ func (s *testPlanSuite) TestSingleRuleTraceStep(c *C) {
 			assertRuleSteps: []assertTraceStep{
 				{
 					assertAction: "agg[6] pushed down across join[5], and join right path becomes agg[8]",
-					assertReason: "agg[%v]'s functions[count(Column#38)] are decomposable with join",
+					assertReason: "agg[6]'s functions[count(Column#38)] are decomposable with join",
 				},
 			},
 		},
@@ -134,11 +134,11 @@ func (s *testPlanSuite) TestSingleRuleTraceStep(c *C) {
 				},
 				{
 					assertAction: "proj[6] is eliminated, and agg[11]'s functions changed into[sum(test.t.c),firstrow(test.t.d)]",
-					assertReason: "projection can be eliminated",
+					assertReason: "Proj[6] is directly below an agg[11] and has no side effects",
 				},
 				{
 					assertAction: "proj[7] is eliminated, and agg[12]'s functions changed into[sum(test.t.a),firstrow(test.t.b)]",
-					assertReason: "projection can be eliminated",
+					assertReason: "Proj[7] is directly below an agg[12] and has no side effects",
 				},
 			},
 		},

diff --git a/planner/core/rule_aggregation_push_down.go b/planner/core/rule_aggregation_push_down.go
@@ -517,7 +517,7 @@ func (*aggregationPushDownSolver) name() string {
 func appendAggPushDownAcrossJoinTraceStep(oldAgg, newAgg *LogicalAggregation, aggFuncs []*aggregation.AggFuncDesc, join *LogicalJoin,
 	childIdx int, opt *logicalOptimizeOp) {
 	reason := func() string {
-		buffer := bytes.NewBufferString("agg[%v]'s functions[")
+		buffer := bytes.NewBufferString(fmt.Sprintf("agg[%v]'s functions[", oldAgg.ID()))
 		for i, aggFunc := range aggFuncs {
 			if i > 0 {
 				buffer.WriteString(",")
@@ -552,7 +552,8 @@ func appendAggPushDownAcrossProjTraceStep(agg *LogicalAggregation, proj *Logical
 		buffer.WriteString("]")
 		return buffer.String()
 	}()
-	opt.appendStepToCurrent(agg.ID(), agg.TP(), "projection can be eliminated", action)
+	reason := fmt.Sprintf("Proj[%v] is directly below an agg[%v] and has no side effects", proj.ID(), agg.ID())
+	opt.appendStepToCurrent(agg.ID(), agg.TP(), reason, action)
 }
 
 func appendAggPushDownAcrossUnionTraceStep(union *LogicalUnionAll, agg *LogicalAggregation, opt *logicalOptimizeOp) {

diff --git a/planner/core/rule_eliminate_projection.go b/planner/core/rule_eliminate_projection.go
@@ -315,7 +315,7 @@ func appendDupProjEliminateTraceStep(parent, child *LogicalProjection, opt *logi
 }
 
 func appendProjEliminateTraceStep(proj *LogicalProjection, opt *logicalOptimizeOp) {
-	reason := fmt.Sprintf("Pro[%v] can be eliminated", proj.ID())
+	reason := fmt.Sprintf("Proj[%v]'s Exprs are all Columns", proj.ID())
 	action := fmt.Sprintf("Proj[%v] is eliminated", proj.ID())
 	opt.appendStepToCurrent(proj.ID(), proj.TP(), reason, action)
 }