Skip to content

Commit

Permalink
planner/core: check the window func arg first before checking window …
Browse files Browse the repository at this point in the history
…specs (#11613) (#11705)
  • Loading branch information
sre-bot authored Aug 13, 2019
1 parent c7ee0dd commit 09e10fa
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 2 deletions.
3 changes: 1 addition & 2 deletions cmd/explaintest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,9 @@ func (t *tester) execute(query query) error {
gotBuf := t.buf.Bytes()[offset:]

buf := make([]byte, t.buf.Len()-offset)
if _, err = t.resultFD.ReadAt(buf, int64(offset)); err != nil {
if _, err = t.resultFD.ReadAt(buf, int64(offset)); !(err == nil || err == io.EOF) {
return errors.Trace(errors.Errorf("run \"%v\" at line %d err, we got \n%s\nbut read result err %s", st.Text(), query.Line, gotBuf, err))
}

if !bytes.Equal(gotBuf, buf) {
return errors.Trace(errors.Errorf("run \"%v\" at line %d err, we need:\n%s\nbut got:\n%s\n", query.Query, query.Line, buf, gotBuf))
}
Expand Down
51 changes: 51 additions & 0 deletions planner/core/logical_plan_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2092,6 +2092,11 @@ func (b *PlanBuilder) buildSelect(ctx context.Context, sel *ast.SelectStmt) (p L
var windowMapper map[*ast.WindowFuncExpr]int
if hasWindowFuncField {
windowFuncs := extractWindowFuncs(sel.Fields.Fields)
// we need to check the func args first before we check the window spec
err := b.checkWindowFuncArgs(ctx, p, windowFuncs, windowAggMap)
if err != nil {
return nil, err
}
groupedFuncs, err := b.groupWindowFuncs(windowFuncs)
if err != nil {
return nil, err
Expand Down Expand Up @@ -2938,6 +2943,35 @@ func (b *PlanBuilder) buildProjectionForWindow(ctx context.Context, p LogicalPla
return proj, propertyItems[:lenPartition], propertyItems[lenPartition:], newArgList, nil
}

func (b *PlanBuilder) buildArgs4WindowFunc(ctx context.Context, p LogicalPlan, args []ast.ExprNode, aggMap map[*ast.AggregateFuncExpr]int) ([]expression.Expression, error) {
b.optFlag |= flagEliminateProjection

newArgList := make([]expression.Expression, 0, len(args))
// use below index for created a new col definition
// it's okay here because we only want to return the args used in window function
newColIndex := 0
for _, arg := range args {
newArg, np, err := b.rewrite(ctx, arg, p, aggMap, true)
if err != nil {
return nil, err
}
p = np
switch newArg.(type) {
case *expression.Column, *expression.Constant:
newArgList = append(newArgList, newArg)
continue
}
col := &expression.Column{
ColName: model.NewCIStr(fmt.Sprintf("%d_proj_window_%d", p.ID(), newColIndex)),
UniqueID: b.ctx.GetSessionVars().AllocPlanColumnID(),
RetType: newArg.GetType(),
}
newColIndex += 1
newArgList = append(newArgList, col)
}
return newArgList, nil
}

func (b *PlanBuilder) buildByItemsForWindow(
ctx context.Context,
p LogicalPlan,
Expand Down Expand Up @@ -3099,6 +3133,23 @@ func (b *PlanBuilder) buildWindowFunctionFrame(ctx context.Context, spec *ast.Wi
return frame, err
}

func (b *PlanBuilder) checkWindowFuncArgs(ctx context.Context, p LogicalPlan, windowFuncExprs []*ast.WindowFuncExpr, windowAggMap map[*ast.AggregateFuncExpr]int) error {
for _, windowFuncExpr := range windowFuncExprs {
args, err := b.buildArgs4WindowFunc(ctx, p, windowFuncExpr.Args, windowAggMap)
if err != nil {
return err
}
desc, err := aggregation.NewWindowFuncDesc(b.ctx, windowFuncExpr.F, args)
if err != nil {
return err
}
if desc == nil {
return ErrWrongArguments.GenWithStackByArgs(strings.ToLower(windowFuncExpr.F))
}
}
return nil
}

func getAllByItems(itemsBuf []*ast.ByItem, spec *ast.WindowSpec) []*ast.ByItem {
itemsBuf = itemsBuf[:0]
if spec.PartitionBy != nil {
Expand Down
8 changes: 8 additions & 0 deletions planner/core/logical_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2485,6 +2485,14 @@ func (s *testPlanSuite) TestWindowFunction(c *C) {
sql: "SELECT NTH_VALUE(a, 1) FROM LAST IGNORE NULLS over (partition by b order by b), a FROM t",
result: "[planner:1235]This version of TiDB doesn't yet support 'IGNORE NULLS'",
},
{
sql: "SELECT NTH_VALUE(fieldA, ATAN(-1)) OVER (w1) AS 'ntile', fieldA, fieldB FROM ( SELECT a AS fieldA, b AS fieldB FROM t ) as te WINDOW w1 AS ( ORDER BY fieldB ASC, fieldA DESC )",
result: "[planner:1210]Incorrect arguments to nth_value",
},
{
sql: "SELECT NTH_VALUE(fieldA, -1) OVER (w1 PARTITION BY fieldB ORDER BY fieldB , fieldA ) AS 'ntile', fieldA, fieldB FROM ( SELECT a AS fieldA, b AS fieldB FROM t ) as temp WINDOW w1 AS ( ORDER BY fieldB ASC, fieldA DESC )",
result: "[planner:1210]Incorrect arguments to nth_value",
},
}

s.Parser.EnableWindowFunc(true)
Expand Down

0 comments on commit 09e10fa

Please sign in to comment.