Skip to content

Commit ca1596d

Browse files
authored
Merge branch 'master' into master
2 parents 987edeb + 0ec7949 commit ca1596d

File tree

7 files changed

+91
-48
lines changed

7 files changed

+91
-48
lines changed

executor/benchmark_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ func buildWindowExecutor(ctx sessionctx.Context, windowFunc string, funcs int, f
527527
default:
528528
args = append(args, partitionBy[0])
529529
}
530-
desc, _ := aggregation.NewWindowFuncDesc(ctx, windowFunc, args)
530+
desc, _ := aggregation.NewWindowFuncDesc(ctx, windowFunc, args, false)
531531

532532
win.WindowFuncDescs = append(win.WindowFuncDescs, desc)
533533
winSchema.Append(&expression.Column{

expression/aggregation/window_func.go

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,33 @@ type WindowFuncDesc struct {
3030
}
3131

3232
// NewWindowFuncDesc creates a window function signature descriptor.
33-
func NewWindowFuncDesc(ctx sessionctx.Context, name string, args []expression.Expression) (*WindowFuncDesc, error) {
34-
switch strings.ToLower(name) {
35-
case ast.WindowFuncNthValue:
36-
val, isNull, ok := expression.GetUint64FromConstant(args[1])
37-
// nth_value does not allow `0`, but allows `null`.
38-
if !ok || (val == 0 && !isNull) {
39-
return nil, nil
40-
}
41-
case ast.WindowFuncNtile:
42-
val, isNull, ok := expression.GetUint64FromConstant(args[0])
43-
// ntile does not allow `0`, but allows `null`.
44-
if !ok || (val == 0 && !isNull) {
45-
return nil, nil
46-
}
47-
case ast.WindowFuncLead, ast.WindowFuncLag:
48-
if len(args) < 2 {
49-
break
50-
}
51-
_, isNull, ok := expression.GetUint64FromConstant(args[1])
52-
if !ok || isNull {
53-
return nil, nil
33+
func NewWindowFuncDesc(ctx sessionctx.Context, name string, args []expression.Expression, skipCheckArgs bool) (*WindowFuncDesc, error) {
34+
// if we are in the prepare statement, skip the params check since it's not been initialized.
35+
if !skipCheckArgs {
36+
switch strings.ToLower(name) {
37+
case ast.WindowFuncNthValue:
38+
val, isNull, ok := expression.GetUint64FromConstant(args[1])
39+
// nth_value does not allow `0`, but allows `null`.
40+
if !ok || (val == 0 && !isNull) {
41+
return nil, nil
42+
}
43+
case ast.WindowFuncNtile:
44+
val, isNull, ok := expression.GetUint64FromConstant(args[0])
45+
// ntile does not allow `0`, but allows `null`.
46+
if !ok || (val == 0 && !isNull) {
47+
return nil, nil
48+
}
49+
case ast.WindowFuncLead, ast.WindowFuncLag:
50+
if len(args) < 2 {
51+
break
52+
}
53+
_, isNull, ok := expression.GetUint64FromConstant(args[1])
54+
if !ok || isNull {
55+
return nil, nil
56+
}
5457
}
5558
}
59+
5660
base, err := newBaseFuncDesc(ctx, name, args)
5761
if err != nil {
5862
return nil, err

expression/util.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,26 @@ func ParamMarkerExpression(ctx sessionctx.Context, v *driver.ParamMarkerExpr, ne
924924
return value, nil
925925
}
926926

927+
// ParamMarkerInPrepareChecker checks whether the given ast tree has paramMarker and is in prepare statement.
928+
type ParamMarkerInPrepareChecker struct {
929+
InPrepareStmt bool
930+
}
931+
932+
// Enter implements Visitor Interface.
933+
func (pc *ParamMarkerInPrepareChecker) Enter(in ast.Node) (out ast.Node, skipChildren bool) {
934+
switch v := in.(type) {
935+
case *driver.ParamMarkerExpr:
936+
pc.InPrepareStmt = !v.InExecute
937+
return v, true
938+
}
939+
return in, false
940+
}
941+
942+
// Leave implements Visitor Interface.
943+
func (pc *ParamMarkerInPrepareChecker) Leave(in ast.Node) (out ast.Node, ok bool) {
944+
return in, true
945+
}
946+
927947
// DisableParseJSONFlag4Expr disables ParseToJSONFlag for `expr` except Column.
928948
// We should not *PARSE* a string as JSON under some scenarios. ParseToJSONFlag
929949
// is 0 for JSON column yet(as well as JSON correlated column), so we can skip

parser/parser.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

parser/parser.y

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8715,7 +8715,7 @@ OptLeadLagInfo:
87158715
}
87168716
| ',' paramMarker OptLLDefault
87178717
{
8718-
args := []ast.ExprNode{ast.NewValueExpr($2, parser.charset, parser.collation)}
8718+
args := []ast.ExprNode{ast.NewParamMarkerExpr(yyS[yypt-1].offset)}
87198719
if $3 != nil {
87208720
args = append(args, $3.(ast.ExprNode))
87218721
}

planner/core/logical_plan_builder.go

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5809,11 +5809,11 @@ func (b *PlanBuilder) buildWindowFunctionFrameBound(ctx context.Context, spec *a
58095809
}
58105810
expr := expression.Constant{Value: val, RetType: boundClause.Expr.GetType()}
58115811

5812-
checker := &paramMarkerInPrepareChecker{}
5812+
checker := &expression.ParamMarkerInPrepareChecker{}
58135813
boundClause.Expr.Accept(checker)
58145814

58155815
// If it has paramMarker and is in prepare stmt. We don't need to eval it since its value is not decided yet.
5816-
if !checker.inPrepareStmt {
5816+
if !checker.InPrepareStmt {
58175817
// Do not raise warnings for truncate.
58185818
oriIgnoreTruncate := b.ctx.GetSessionVars().StmtCtx.IgnoreTruncate
58195819
b.ctx.GetSessionVars().StmtCtx.IgnoreTruncate = true
@@ -5862,26 +5862,6 @@ func (b *PlanBuilder) buildWindowFunctionFrameBound(ctx context.Context, spec *a
58625862
return bound, nil
58635863
}
58645864

5865-
// paramMarkerInPrepareChecker checks whether the given ast tree has paramMarker and is in prepare statement.
5866-
type paramMarkerInPrepareChecker struct {
5867-
inPrepareStmt bool
5868-
}
5869-
5870-
// Enter implements Visitor Interface.
5871-
func (pc *paramMarkerInPrepareChecker) Enter(in ast.Node) (out ast.Node, skipChildren bool) {
5872-
switch v := in.(type) {
5873-
case *driver.ParamMarkerExpr:
5874-
pc.inPrepareStmt = !v.InExecute
5875-
return v, true
5876-
}
5877-
return in, false
5878-
}
5879-
5880-
// Leave implements Visitor Interface.
5881-
func (pc *paramMarkerInPrepareChecker) Leave(in ast.Node) (out ast.Node, ok bool) {
5882-
return in, true
5883-
}
5884-
58855865
// buildWindowFunctionFrame builds the window function frames.
58865866
// See https://dev.mysql.com/doc/refman/8.0/en/window-functions-frames.html
58875867
func (b *PlanBuilder) buildWindowFunctionFrame(ctx context.Context, spec *ast.WindowSpec, orderByItems []property.SortItem) (*WindowFrame, error) {
@@ -5900,6 +5880,7 @@ func (b *PlanBuilder) buildWindowFunctionFrame(ctx context.Context, spec *ast.Wi
59005880
}
59015881

59025882
func (b *PlanBuilder) checkWindowFuncArgs(ctx context.Context, p LogicalPlan, windowFuncExprs []*ast.WindowFuncExpr, windowAggMap map[*ast.AggregateFuncExpr]int) error {
5883+
checker := &expression.ParamMarkerInPrepareChecker{}
59035884
for _, windowFuncExpr := range windowFuncExprs {
59045885
if strings.ToLower(windowFuncExpr.F) == ast.AggFuncGroupConcat {
59055886
return ErrNotSupportedYet.GenWithStackByArgs("group_concat as window function")
@@ -5908,7 +5889,11 @@ func (b *PlanBuilder) checkWindowFuncArgs(ctx context.Context, p LogicalPlan, wi
59085889
if err != nil {
59095890
return err
59105891
}
5911-
desc, err := aggregation.NewWindowFuncDesc(b.ctx, windowFuncExpr.F, args)
5892+
checker.InPrepareStmt = false
5893+
for _, expr := range windowFuncExpr.Args {
5894+
expr.Accept(checker)
5895+
}
5896+
desc, err := aggregation.NewWindowFuncDesc(b.ctx, windowFuncExpr.F, args, checker.InPrepareStmt)
59125897
if err != nil {
59135898
return err
59145899
}
@@ -6018,8 +6003,13 @@ func (b *PlanBuilder) buildWindowFunctions(ctx context.Context, p LogicalPlan, g
60186003
schema := np.Schema().Clone()
60196004
descs := make([]*aggregation.WindowFuncDesc, 0, len(funcs))
60206005
preArgs := 0
6006+
checker := &expression.ParamMarkerInPrepareChecker{}
60216007
for _, windowFunc := range funcs {
6022-
desc, err := aggregation.NewWindowFuncDesc(b.ctx, windowFunc.F, args[preArgs:preArgs+len(windowFunc.Args)])
6008+
checker.InPrepareStmt = false
6009+
for _, expr := range windowFunc.Args {
6010+
expr.Accept(checker)
6011+
}
6012+
desc, err := aggregation.NewWindowFuncDesc(b.ctx, windowFunc.F, args[preArgs:preArgs+len(windowFunc.Args)], checker.InPrepareStmt)
60236013
if err != nil {
60246014
return nil, nil, err
60256015
}

planner/core/prepare_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,35 @@ func TestPrepareWithWindowFunction(t *testing.T) {
10281028
tk.MustQuery("execute stmt2 using @a, @b").Check(testkit.Rows("0", "0"))
10291029
}
10301030

1031+
func TestPrepareWindowFunctionWithoutParamsCheck(t *testing.T) {
1032+
store, clean := testkit.CreateMockStore(t)
1033+
defer clean()
1034+
tk := testkit.NewTestKit(t, store)
1035+
tk.MustExec("set @@tidb_enable_window_function = 1")
1036+
defer tk.MustExec("set @@tidb_enable_window_function = 0")
1037+
tk.MustExec("use test")
1038+
tk.MustExec("CREATE TABLE t1 (d DOUBLE, id INT, sex CHAR(1), n INT NOT NULL AUTO_INCREMENT, PRIMARY KEY(n));")
1039+
tk.MustExec("insert into t1(d, id, sex) values (1.0, 1, 'M'),(2.0, 2, 'F'),(3.0, 3, 'F'),(4.0, 4, 'F'),(5.0, 5, 'M')")
1040+
// prepare phase, we don't check the window function args.
1041+
tk.MustExec("prepare p from \"select id, sex, lead(id, ?) over () from t1\"")
1042+
// execute phase, we do check the window function args (param is initialized).
1043+
err := tk.ExecToErr("execute p using @p1")
1044+
require.NotNil(t, err)
1045+
require.EqualError(t, err, "[planner:1210]Incorrect arguments to lead")
1046+
tk.MustExec("set @p1 = 3")
1047+
// execute phase, we do check the window function args (param is valid).
1048+
tk.MustQuery("execute p using @p1").Check(testkit.Rows("1 M 4", "2 F 5", "3 F <nil>", "4 F <nil>", "5 M <nil>"))
1049+
1050+
// execute phase, we do check the window function args (param is invalid).
1051+
tk.MustExec("PREPARE p FROM \"SELECT id, sex, LEAD(id, ?) OVER (), ntile(?) over() FROM t1\"")
1052+
tk.MustExec("set @p2 = 3")
1053+
tk.MustQuery("execute p using @p1, @p2").Check(testkit.Rows("1 M 4 1", "2 F 5 1", "3 F <nil> 2", "4 F <nil> 2", "5 M <nil> 3"))
1054+
tk.MustExec("set @p2 = 0") // ntile doesn't allow param to be 0
1055+
err = tk.ExecToErr("execute p using @p1, @p2")
1056+
require.NotNil(t, err)
1057+
require.EqualError(t, err, "[planner:1210]Incorrect arguments to ntile")
1058+
}
1059+
10311060
func TestPrepareWithSnapshot(t *testing.T) {
10321061
store, clean := testkit.CreateMockStore(t)
10331062
defer clean()

0 commit comments

Comments
 (0)