Skip to content

Commit

Permalink
planner/core: do not change window ast when building plan (#12… (#12934)
Browse files Browse the repository at this point in the history
  • Loading branch information
XuHuaiyu authored and zz-jason committed Oct 25, 2019
1 parent 66150ce commit c4befe6
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 20 deletions.
10 changes: 6 additions & 4 deletions planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3350,7 +3350,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
}
Expand All @@ -3371,11 +3374,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
}
Expand Down Expand Up @@ -3483,7 +3486,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)
Expand Down
50 changes: 34 additions & 16 deletions planner/core/logical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -2512,31 +2513,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),
hintProcessor: &BlockHintProcessor{},
}
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 {
Expand Down

0 comments on commit c4befe6

Please sign in to comment.