From e4911f2b9a4af7f2c896ecbbc5b5872d7f58813f Mon Sep 17 00:00:00 2001
From: Haibin Xie <lambdax.tyler@gmail.com>
Date: Thu, 26 Sep 2019 14:16:31 +0800
Subject: [PATCH 1/2] planner/core: do not change window ast when building plan
 (#12347)

---
 planner/core/logical_plan_builder.go | 10 +++---
 planner/core/logical_plan_test.go    | 49 +++++++++++++++++++---------
 2 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go
index c6e0136f97c4b..7e4d40e96b301 100644
--- a/planner/core/logical_plan_builder.go
+++ b/planner/core/logical_plan_builder.go
@@ -3301,7 +3301,10 @@ func (b *PlanBuilder) checkOriginWindowSpecs(funcs []*ast.WindowFuncExpr, orderB
 		if f.FromLast {
 			return ErrNotSupportedYet.GenWithStackByArgs("FROM LAST")
 		}
-		spec := f.Spec
+		spec := &f.Spec
+		if f.Spec.Name.L != "" {
+			spec = b.windowSpecs[f.Spec.Name.L]
+		}
 		if spec.Frame == nil {
 			continue
 		}
@@ -3322,11 +3325,11 @@ func (b *PlanBuilder) checkOriginWindowSpecs(funcs []*ast.WindowFuncExpr, orderB
 			return ErrWindowFrameIllegal.GenWithStackByArgs(getWindowName(spec.Name.O))
 		}
 
-		err := b.checkOriginWindowFrameBound(&start, &spec, orderByItems)
+		err := b.checkOriginWindowFrameBound(&start, spec, orderByItems)
 		if err != nil {
 			return err
 		}
-		err = b.checkOriginWindowFrameBound(&end, &spec, orderByItems)
+		err = b.checkOriginWindowFrameBound(&end, spec, orderByItems)
 		if err != nil {
 			return err
 		}
@@ -3434,7 +3437,6 @@ func (b *PlanBuilder) groupWindowFuncs(windowFuncs []*ast.WindowFuncExpr) (map[*
 		if !ok {
 			return nil, ErrWindowNoSuchWindow.GenWithStackByArgs(windowFunc.Spec.Name.O)
 		}
-		windowFunc.Spec = *spec
 		newSpec, updated := b.handleDefaultFrame(spec, windowFunc.F)
 		if !updated {
 			groupedWindow[spec] = append(groupedWindow[spec], windowFunc)
diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go
index 511c79069070b..649926d5c84fb 100644
--- a/planner/core/logical_plan_test.go
+++ b/planner/core/logical_plan_test.go
@@ -23,6 +23,7 @@ import (
 	. "github.com/pingcap/check"
 	"github.com/pingcap/parser"
 	"github.com/pingcap/parser/ast"
+	"github.com/pingcap/parser/format"
 	"github.com/pingcap/parser/model"
 	"github.com/pingcap/parser/mysql"
 	"github.com/pingcap/parser/terror"
@@ -2511,30 +2512,48 @@ func (s *testPlanSuite) TestWindowFunction(c *C) {
 	ctx := context.TODO()
 	for i, tt := range tests {
 		comment := Commentf("case:%v sql:%s", i, tt.sql)
-		stmt, err := s.ParseOneStmt(tt.sql, "", "")
-		c.Assert(err, IsNil, comment)
-		Preprocess(s.ctx, stmt, s.is)
-		builder := &PlanBuilder{
-			ctx:       MockContext(),
-			is:        s.is,
-			colMapper: make(map[*ast.ColumnNameExpr]int),
-		}
-		p, err := builder.Build(ctx, stmt)
+		p, stmt, err := s.optimize(ctx, tt.sql)
 		if err != nil {
 			c.Assert(err.Error(), Equals, tt.result, comment)
 			continue
 		}
+		c.Assert(ToString(p), Equals, tt.result, comment)
+
+		var sb strings.Builder
+		// After restore, the result should be the same.
+		err = stmt.Restore(format.NewRestoreCtx(format.DefaultRestoreFlags, &sb))
 		c.Assert(err, IsNil)
-		p, err = logicalOptimize(ctx, builder.optFlag, p.(LogicalPlan))
-		c.Assert(err, IsNil)
-		lp, ok := p.(LogicalPlan)
-		c.Assert(ok, IsTrue)
-		p, err = physicalOptimize(lp)
-		c.Assert(err, IsNil)
+		p, _, err = s.optimize(ctx, sb.String())
+		if err != nil {
+			c.Assert(err.Error(), Equals, tt.result, comment)
+			continue
+		}
 		c.Assert(ToString(p), Equals, tt.result, comment)
 	}
 }
 
+func (s *testPlanSuite) optimize(ctx context.Context, sql string) (PhysicalPlan, ast.Node, error) {
+	stmt, err := s.ParseOneStmt(sql, "", "")
+	if err != nil {
+		return nil, nil, err
+	}
+	err = Preprocess(s.ctx, stmt, s.is)
+	if err != nil {
+		return nil, nil, err
+	}
+	builder := NewPlanBuilder(MockContext(), s.is, &BlockHintProcessor{})
+	p, err := builder.Build(ctx, stmt)
+	if err != nil {
+		return nil, nil, err
+	}
+	p, err = logicalOptimize(ctx, builder.optFlag, p.(LogicalPlan))
+	if err != nil {
+		return nil, nil, err
+	}
+	p, err = physicalOptimize(p.(LogicalPlan))
+	return p.(PhysicalPlan), stmt, err
+}
+
 func byItemsToProperty(byItems []*ByItems) *property.PhysicalProperty {
 	pp := &property.PhysicalProperty{}
 	for _, item := range byItems {

From ec772cb192d238964aa5feb73defbf072c9a7549 Mon Sep 17 00:00:00 2001
From: xuhuaiyu <391585975@qq.com>
Date: Fri, 25 Oct 2019 16:05:12 +0800
Subject: [PATCH 2/2] remove useless code

---
 planner/core/logical_plan_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go
index 649926d5c84fb..897a104097c8f 100644
--- a/planner/core/logical_plan_test.go
+++ b/planner/core/logical_plan_test.go
@@ -2541,7 +2541,7 @@ func (s *testPlanSuite) optimize(ctx context.Context, sql string) (PhysicalPlan,
 	if err != nil {
 		return nil, nil, err
 	}
-	builder := NewPlanBuilder(MockContext(), s.is, &BlockHintProcessor{})
+	builder := NewPlanBuilder(MockContext(), s.is)
 	p, err := builder.Build(ctx, stmt)
 	if err != nil {
 		return nil, nil, err