Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -2099,6 +2099,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 @@ -3056,6 +3061,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) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could buildArgs4WindowFunc also be used in buildProjectionForWindow?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. if we use that, we need to change func buildArgs4WindowFunc to add some projection information into the plan. and then you may find the two methods are same. if so, I think it's much better just call same method instead of two methods

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 @@ -3220,6 +3254,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 @@ -2469,6 +2469,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