Revert "plan: covert max/min to Limit + Sort operators (pingcap#5105)"

This reverts commit 4b669b7.

executor/index_lookup_join.go

@@ -276,9 +276,9 @@ func (e *IndexLookUpJoin) fetchSortedInners(requestRows [][]types.Datum) error {
 			break
 		}
 
-		matched, err1 := expression.EvalBool(e.innerFilter, innerRow, e.ctx)
-		if err1 != nil {
-			return errors.Trace(err1)
+		matched, err := expression.EvalBool(e.innerFilter, innerRow, e.ctx)
+		if err != nil {
+			return errors.Trace(err)
 		} else if matched {
 			e.innerOrderedRows.rows = append(e.innerOrderedRows.rows, innerRow)
 		}
@@ -288,9 +288,9 @@ func (e *IndexLookUpJoin) fetchSortedInners(requestRows [][]types.Datum) error {
 	innerJoinKey := e.buffer4JoinKey[:0]
 	for _, innerRow := range e.innerOrderedRows.rows {
 		for _, innerKey := range e.innerKeys {
-			innerDatum, err1 := innerKey.Eval(innerRow)
-			if err1 != nil {
-				return errors.Trace(err1)
+			innerDatum, err := innerKey.Eval(innerRow)
+			if err != nil {
+				return errors.Trace(err)
 			}
 			innerJoinKey = append(innerJoinKey, innerDatum)
 		}

executor/join.go

@@ -348,9 +348,9 @@ func (e *HashJoinExec) joinOuterRow(workerID int, outerRow Row, resultBuffer *ex
 
 	innerRows := make([]Row, 0, len(values))
 	for _, value := range values {
-		innerRow, err1 := e.decodeRow(value)
-		if err1 != nil {
-			resultBuffer.err = errors.Trace(err1)
+		innerRow, err := e.decodeRow(value)
+		if err != nil {
+			resultBuffer.err = errors.Trace(err)
 			return false
 		}
 		innerRows = append(innerRows, innerRow)

plan/logical_plan_builder.go

@@ -63,10 +63,6 @@ func (p *LogicalAggregation) collectGroupByColumns() {
 func (b *planBuilder) buildAggregation(p LogicalPlan, aggFuncList []*ast.AggregateFuncExpr, gbyItems []expression.Expression) (LogicalPlan, map[int]int) {
 	b.optFlag = b.optFlag | flagBuildKeyInfo
 	b.optFlag = b.optFlag | flagAggregationOptimize
-	// We may apply aggregation eliminate optimization.
-	// So we add the flagAggEliminate to try to convert max/min to topn and flagPushDownTopN to handle the newly added topn operator.
-	b.optFlag = b.optFlag | flagAggEliminate
-	b.optFlag = b.optFlag | flagPushDownTopN
 
 	agg := LogicalAggregation{AggFuncs: make([]aggregation.Aggregation, 0, len(aggFuncList))}.init(b.ctx)
 	schema := expression.NewSchema(make([]*expression.Column, 0, len(aggFuncList)+p.Schema().Len())...)

plan/logical_plan_test.go

@@ -1595,56 +1595,3 @@ func (s *testPlanSuite) TestNameResolver(c *C) {
 		}
 	}
 }
-
-func (s *testPlanSuite) TestAggEliminater(c *C) {
-	defer func() {
-		testleak.AfterTest(c)()
-	}()
-	tests := []struct {
-		sql  string
-		best string
-	}{
-		// Max to Limit + Sort-Desc.
-		{
-			sql:  "select max(a) from t;",
-			best: "DataScan(t)->TopN([test.t.a true],0,1)->Projection->Projection",
-		},
-		// Min to Limit + Sort.
-		{
-			sql:  "select min(a) from t;",
-			best: "DataScan(t)->TopN([test.t.a],0,1)->Projection->Projection",
-		},
-		// Do nothing to max + firstrow.
-		{
-			sql:  "select max(a), b from t;",
-			best: "DataScan(t)->Aggr(max(test.t.a),firstrow(test.t.b))->Projection",
-		},
-		// Do nothing to max+min.
-		{
-			sql:  "select max(a), min(a) from t;",
-			best: "DataScan(t)->Aggr(max(test.t.a),min(test.t.a))->Projection",
-		},
-		// Do nothing to max with groupby.
-		{
-			sql:  "select max(a) from t group by b;",
-			best: "DataScan(t)->Aggr(max(test.t.a))->Projection",
-		},
-	}
-
-	for _, tt := range tests {
-		comment := Commentf("for %s", tt.sql)
-		stmt, err := s.ParseOneStmt(tt.sql, "", "")
-		c.Assert(err, IsNil, comment)
-		Preprocess(s.ctx, stmt, s.is, false)
-		builder := &planBuilder{
-			ctx:       mockContext(),
-			is:        s.is,
-			colMapper: make(map[*ast.ColumnNameExpr]int),
-		}
-		p := builder.build(stmt).(LogicalPlan)
-		c.Assert(builder.err, IsNil)
-		p, err = logicalOptimize(builder.optFlag, p.(LogicalPlan), builder.ctx)
-		c.Assert(err, IsNil)
-		c.Assert(ToString(p), Equals, tt.best, comment)
-	}
-}

plan/optimizer.go

@@ -36,7 +36,6 @@ const (
 	flagDecorrelate
 	flagPredicatePushDown
 	flagAggregationOptimize
-	flagAggEliminate // Keep AggEliminater before PushDownTopN, for it will add a TopN operator.
 	flagPushDownTopN
 )
 
@@ -47,7 +46,6 @@ var optRuleList = []logicalOptRule{
 	&decorrelateSolver{},
 	&ppdSolver{},
 	&aggregationOptimizer{},
-	&aggEliminater{},
 	&pushDownTopNOptimizer{},
 }