plan: support ? in Order By / Group By / Limit Offset clauses (#8206)

executor/prepared.go

@@ -26,7 +26,6 @@ import (
 	plannercore "github.com/pingcap/tidb/planner/core"
 	"github.com/pingcap/tidb/sessionctx"
 	"github.com/pingcap/tidb/sessionctx/variable"
-	"github.com/pingcap/tidb/types"
 	driver "github.com/pingcap/tidb/types/parser_driver"
 	"github.com/pingcap/tidb/util/chunk"
 	"github.com/pingcap/tidb/util/sqlexec"
@@ -166,7 +165,7 @@ func (e *PrepareExec) Next(ctx context.Context, chk *chunk.Chunk) error {
 
 	// We try to build the real statement of preparedStmt.
 	for i := range prepared.Params {
-		prepared.Params[i].(*driver.ParamMarkerExpr).Datum = types.NewIntDatum(0)
+		prepared.Params[i].(*driver.ParamMarkerExpr).Datum.SetNull()
 	}
 	var p plannercore.Plan
 	p, err = plannercore.BuildLogicalPlan(e.ctx, stmt, e.is)

executor/prepared_test.go

@@ -366,3 +366,54 @@ func generateBatchSQL(paramCount int) (sql string, paramSlice []interface{}) {
 	}
 	return "insert into t values " + strings.Join(placeholders, ","), params
 }
+
+func (s *testSuite) TestPreparedIssue8153(c *C) {
+	orgEnable := plannercore.PreparedPlanCacheEnabled()
+	orgCapacity := plannercore.PreparedPlanCacheCapacity
+	defer func() {
+		plannercore.SetPreparedPlanCache(orgEnable)
+		plannercore.PreparedPlanCacheCapacity = orgCapacity
+	}()
+	flags := []bool{false, true}
+	for _, flag := range flags {
+		var err error
+		plannercore.SetPreparedPlanCache(flag)
+		plannercore.PreparedPlanCacheCapacity = 100
+		tk := testkit.NewTestKit(c, s.store)
+		tk.MustExec("use test")
+		tk.MustExec("drop table if exists t")
+		tk.MustExec("create table t (a int, b int)")
+		tk.MustExec("insert into t (a, b) values (1,3), (2,2), (3,1)")
+
+		tk.MustExec(`prepare stmt from 'select * from t order by ? asc'`)
+		r := tk.MustQuery(`execute stmt using @param;`)
+		r.Check(testkit.Rows("1 3", "2 2", "3 1"))
+
+		tk.MustExec(`set @param = 1`)
+		r = tk.MustQuery(`execute stmt using @param;`)
+		r.Check(testkit.Rows("1 3", "2 2", "3 1"))
+
+		tk.MustExec(`set @param = 2`)
+		r = tk.MustQuery(`execute stmt using @param;`)
+		r.Check(testkit.Rows("3 1", "2 2", "1 3"))
+
+		tk.MustExec(`set @param = 3`)
+		_, err = tk.Exec(`execute stmt using @param;`)
+		c.Assert(err.Error(), Equals, "[planner:1054]Unknown column '?' in 'order clause'")
+
+		tk.MustExec(`set @param = '##'`)
+		r = tk.MustQuery(`execute stmt using @param;`)
+		r.Check(testkit.Rows("1 3", "2 2", "3 1"))
+
+		tk.MustExec("insert into t (a, b) values (1,1), (1,2), (2,1), (2,3), (3,2), (3,3)")
+		tk.MustExec(`prepare stmt from 'select ?, sum(a) from t group by ?'`)
+
+		tk.MustExec(`set @a=1,@b=1`)
+		r = tk.MustQuery(`execute stmt using @a,@b;`)
+		r.Check(testkit.Rows("1 18"))
+
+		tk.MustExec(`set @a=1,@b=2`)
+		_, err = tk.Exec(`execute stmt using @a,@b;`)
+		c.Assert(err.Error(), Equals, "[planner:1056]Can't group on 'sum(a)'")
+	}
+}

expression/simple_rewriter.go

@@ -156,9 +156,11 @@ func (sr *simpleRewriter) Leave(originInNode ast.Node) (retNode ast.Node, ok boo
 			sr.inToExpression(len(v.List), v.Not, &v.Type)
 		}
 	case *driver.ParamMarkerExpr:
-		tp := types.NewFieldType(mysql.TypeUnspecified)
-		types.DefaultParamTypeForValue(v.GetValue(), tp)
-		value := &Constant{Value: v.ValueExpr.Datum, RetType: tp}
+		var value Expression
+		value, sr.err = GetParamExpression(sr.ctx, v)
+		if sr.err != nil {
+			return retNode, false
+		}
 		sr.push(value)
 	case *ast.RowExpr:
 		sr.rowToScalarFunc(v)

expression/util.go

@@ -26,6 +26,7 @@ import (
 	"github.com/pingcap/parser/terror"
 	"github.com/pingcap/tidb/sessionctx"
 	"github.com/pingcap/tidb/types"
+	driver "github.com/pingcap/tidb/types/parser_driver"
 	"github.com/pingcap/tidb/util/chunk"
 	"github.com/pingcap/tidb/util/hack"
 	"golang.org/x/tools/container/intsets"
@@ -555,3 +556,74 @@ func DisableParseJSONFlag4Expr(expr Expression) {
 	}
 	expr.GetType().Flag &= ^mysql.ParseToJSONFlag
 }
+
+// DatumToConstant generates a Constant expression from a Datum.
+func DatumToConstant(d types.Datum, tp byte) *Constant {
+	return &Constant{Value: d, RetType: types.NewFieldType(tp)}
+}
+
+// GetParamExpression generate a getparam function expression.
+func GetParamExpression(ctx sessionctx.Context, v *driver.ParamMarkerExpr) (Expression, error) {
+	useCache := ctx.GetSessionVars().StmtCtx.UseCache
+	tp := types.NewFieldType(mysql.TypeUnspecified)
+	types.DefaultParamTypeForValue(v.GetValue(), tp)
+	value := &Constant{Value: v.Datum, RetType: tp}
+	if useCache {
+		f, err := NewFunctionBase(ctx, ast.GetParam, &v.Type,
+			DatumToConstant(types.NewIntDatum(int64(v.Order)), mysql.TypeLonglong))
+		if err != nil {
+			return nil, errors.Trace(err)
+		}
+		f.GetType().Tp = v.Type.Tp
+		value.DeferredExpr = f
+	}
+	return value, nil
+}
+
+// ConstructPositionExpr constructs PositionExpr with the given ParamMarkerExpr.
+func ConstructPositionExpr(p *driver.ParamMarkerExpr) *ast.PositionExpr {
+	return &ast.PositionExpr{P: p}
+}
+
+// PosFromPositionExpr generates a position value from PositionExpr.
+func PosFromPositionExpr(ctx sessionctx.Context, v *ast.PositionExpr) (int, bool, error) {
+	if v.P == nil {
+		return v.N, false, nil
+	}
+	value, err := GetParamExpression(ctx, v.P.(*driver.ParamMarkerExpr))
+	if err != nil {
+		return 0, true, err
+	}
+	pos, isNull, err := GetIntFromConstant(ctx, value)
+	if err != nil || isNull {
+		return 0, true, errors.Trace(err)
+	}
+	return pos, false, nil
+}
+
+// GetStringFromConstant gets a string value from the Constant expression.
+func GetStringFromConstant(ctx sessionctx.Context, value Expression) (string, bool, error) {
+	con, ok := value.(*Constant)
+	if !ok {
+		err := errors.Errorf("Not a Constant expression %+v", value)
+		return "", true, errors.Trace(err)
+	}
+	str, isNull, err := con.EvalString(ctx, chunk.Row{})
+	if err != nil || isNull {
+		return "", true, errors.Trace(err)
+	}
+	return str, false, nil
+}
+
+// GetIntFromConstant gets an interger value from the Constant expression.
+func GetIntFromConstant(ctx sessionctx.Context, value Expression) (int, bool, error) {
+	str, isNull, err := GetStringFromConstant(ctx, value)
+	if err != nil || isNull {
+		return 0, true, errors.Trace(err)
+	}
+	intNum, err := strconv.Atoi(str)
+	if err != nil {
+		return 0, true, nil
+	}
+	return intNum, false, nil
+}

go.mod

@@ -52,7 +52,7 @@ require (
 	github.com/pingcap/goleveldb v0.0.0-20171020084629-8d44bfdf1030
 	github.com/pingcap/kvproto v0.0.0-20190826051950-fc8799546726
 	github.com/pingcap/log v0.0.0-20190307075452-bd41d9273596
-	github.com/pingcap/parser v0.0.0-20190910040957-e998b3c52469
+	github.com/pingcap/parser v0.0.0-20191014023410-e7801c924a44
 	github.com/pingcap/pd v2.1.12+incompatible
 	github.com/pingcap/tidb-tools v2.1.3-0.20190116051332-34c808eef588+incompatible
 	github.com/pingcap/tipb v0.0.0-20180910045846-371b48b15d93
@@ -72,9 +72,9 @@ require (
 	go.uber.org/zap v1.9.1
 	golang.org/x/crypto v0.0.0-20180503215945-1f94bef427e3 // indirect
 	golang.org/x/net v0.0.0-20180906233101-161cd47e91fd
-	golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f // indirect
-	golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e // indirect
-	golang.org/x/text v0.3.0
+	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect
+	golang.org/x/sys v0.0.0-20191010194322-b09406accb47 // indirect
+	golang.org/x/text v0.3.2
 	golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 // indirect
 	golang.org/x/tools v0.0.0-20181105230042-78dc5bac0cac
 	google.golang.org/genproto v0.0.0-20180427144745-86e600f69ee4 // indirect

go.sum

@@ -109,8 +109,8 @@ github.com/pingcap/kvproto v0.0.0-20190826051950-fc8799546726 h1:AzGIEmaYVYMtmki
 github.com/pingcap/kvproto v0.0.0-20190826051950-fc8799546726/go.mod h1:0gwbe1F2iBIjuQ9AH0DbQhL+Dpr5GofU8fgYyXk+ykk=
 github.com/pingcap/log v0.0.0-20190307075452-bd41d9273596 h1:t2OQTpPJnrPDGlvA+3FwJptMTt6MEPdzK1Wt99oaefQ=
 github.com/pingcap/log v0.0.0-20190307075452-bd41d9273596/go.mod h1:WpHUKhNZ18v116SvGrmjkA9CBhYmuUTKL+p8JC9ANEw=
-github.com/pingcap/parser v0.0.0-20190910040957-e998b3c52469 h1:JS/p4qMInVXTyV0kjFz+n0DBGn/n1T0cZDjEYHdTQow=
-github.com/pingcap/parser v0.0.0-20190910040957-e998b3c52469/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
+github.com/pingcap/parser v0.0.0-20191014023410-e7801c924a44 h1:DprW0H6iFwLI3YkS/b2rqc5zvpu//myquMUKMzL26SE=
+github.com/pingcap/parser v0.0.0-20191014023410-e7801c924a44/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
 github.com/pingcap/pd v2.1.12+incompatible h1:6N3LBxx2aSZqT+IWEG730EDNDttP7dXO8J6yvBh+HXw=
 github.com/pingcap/pd v2.1.12+incompatible/go.mod h1:nD3+EoYes4+aNNODO99ES59V83MZSI+dFbhyr667a0E=
 github.com/pingcap/tidb-tools v2.1.3-0.20190116051332-34c808eef588+incompatible h1:e9Gi/LP9181HT3gBfSOeSBA+5JfemuE4aEAhqNgoE4k=
@@ -161,14 +161,15 @@ golang.org/x/crypto v0.0.0-20180503215945-1f94bef427e3 h1:+U/hI4i24Enhs+2BEMCN7w
 golang.org/x/crypto v0.0.0-20180503215945-1f94bef427e3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd h1:nTDtHvHSdCn1m6ITfMRqtOd/9+7a3s8RBNOZ3eYZzJA=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
-golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f h1:wMNYb4v58l5UBM7MYRLPG6ZhfOqbKu7X5eyFl8ZhKvA=
-golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e h1:o3PsSEY8E4eXWkXrIP9YJALUkVZqzHJT5DOasTyn8Vs=
-golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
-golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
-golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sys v0.0.0-20191010194322-b09406accb47 h1:/XfQ9z7ib8eEJX2hdgFTZJ/ntt0swNk5oYBziWeTCvY=
+golang.org/x/sys v0.0.0-20191010194322-b09406accb47/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/text v0.3.2 h1:tW2bmiBqwgJj/UpqtC8EpXEZVYOwU0yG4iWbprSVAcs=
+golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 h1:+DCIGbF/swA92ohVg0//6X2IVY3KZs6p9mix0ziNYJM=
 golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
+golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20181105230042-78dc5bac0cac h1:0Nb35Izc6T6Yz1iGmRc4cg14cxRaFjbjD4hWFI6JNJ8=
 golang.org/x/tools v0.0.0-20181105230042-78dc5bac0cac/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 google.golang.org/genproto v0.0.0-20180427144745-86e600f69ee4 h1:0rk3/gV3HbvCeUzVMhdxV3TEVKMVPDnayjN7sYRmcxY=

planner/core/cacheable_checker.go

@@ -51,6 +51,20 @@ func (checker *cacheableChecker) Enter(in ast.Node) (out ast.Node, skipChildren
 			checker.cacheable = false
 			return in, true
 		}
+	case *ast.OrderByClause:
+		for _, item := range node.Items {
+			if _, isParamMarker := item.Expr.(*driver.ParamMarkerExpr); isParamMarker {
+				checker.cacheable = false
+				return in, true
+			}
+		}
+	case *ast.GroupByClause:
+		for _, item := range node.Items {
+			if _, isParamMarker := item.Expr.(*driver.ParamMarkerExpr); isParamMarker {
+				checker.cacheable = false
+				return in, true
+			}
+		}
 	case *ast.Limit:
 		if node.Count != nil {
 			if _, isParamMarker := node.Count.(*driver.ParamMarkerExpr); isParamMarker {

planner/core/cacheable_checker_test.go

@@ -87,4 +87,18 @@ func (s *testCacheableSuite) TestCacheable(c *C) {
 		Limit: limitStmt,
 	}
 	c.Assert(Cacheable(stmt), IsTrue)
+
+	paramExpr := &driver.ParamMarkerExpr{}
+	orderByClause := &ast.OrderByClause{Items: []*ast.ByItem{{Expr: paramExpr}}}
+	stmt = &ast.SelectStmt{
+		OrderBy: orderByClause,
+	}
+	c.Assert(Cacheable(stmt), IsFalse)
+
+	valExpr := &driver.ValueExpr{}
+	orderByClause = &ast.OrderByClause{Items: []*ast.ByItem{{Expr: valExpr}}}
+	stmt = &ast.SelectStmt{
+		OrderBy: orderByClause,
+	}
+	c.Assert(Cacheable(stmt), IsTrue)
 }

planner/core/errors.go

@@ -28,6 +28,7 @@ const (
 
 	codeWrongUsage                   = mysql.ErrWrongUsage
 	codeAmbiguous                    = mysql.ErrNonUniq
+	codeUnknown                      = mysql.ErrUnknown
 	codeUnknownColumn                = mysql.ErrBadField
 	codeUnknownTable                 = mysql.ErrUnknownTable
 	codeWrongArguments               = mysql.ErrWrongArguments
@@ -65,6 +66,7 @@ var (
 
 	ErrWrongUsage                   = terror.ClassOptimizer.New(codeWrongUsage, mysql.MySQLErrName[mysql.ErrWrongUsage])
 	ErrAmbiguous                    = terror.ClassOptimizer.New(codeAmbiguous, mysql.MySQLErrName[mysql.ErrNonUniq])
+	ErrUnknown                      = terror.ClassOptimizer.New(codeUnknown, mysql.MySQLErrName[mysql.ErrUnknown])
 	ErrUnknownColumn                = terror.ClassOptimizer.New(codeUnknownColumn, mysql.MySQLErrName[mysql.ErrBadField])
 	ErrUnknownTable                 = terror.ClassOptimizer.New(codeUnknownTable, mysql.MySQLErrName[mysql.ErrUnknownTable])
 	ErrWrongArguments               = terror.ClassOptimizer.New(codeWrongArguments, mysql.MySQLErrName[mysql.ErrWrongArguments])

planner/core/expression_rewriter.go

@@ -820,11 +820,10 @@ func (er *expressionRewriter) Leave(originInNode ast.Node) (retNode ast.Node, ok
 		value := &expression.Constant{Value: v.Datum, RetType: &v.Type}
 		er.ctxStack = append(er.ctxStack, value)
 	case *driver.ParamMarkerExpr:
-		tp := types.NewFieldType(mysql.TypeUnspecified)
-		types.DefaultParamTypeForValue(v.GetValue(), tp)
-		value := &expression.Constant{Value: v.Datum, RetType: tp}
-		if er.useCache() {
-			value.DeferredExpr = er.getParamExpression(v)
+		var value expression.Expression
+		value, er.err = expression.GetParamExpression(er.ctx, v)
+		if er.err != nil {
+			return retNode, false
 		}
 		er.ctxStack = append(er.ctxStack, value)
 	case *ast.VariableExpr:
@@ -1044,10 +1043,26 @@ func (er *expressionRewriter) isNullToExpression(v *ast.IsNullExpr) {
 }
 
 func (er *expressionRewriter) positionToScalarFunc(v *ast.PositionExpr) {
-	if v.N > 0 && v.N <= er.schema.Len() {
-		er.ctxStack = append(er.ctxStack, er.schema.Columns[v.N-1])
+	pos := v.N
+	str := strconv.Itoa(pos)
+	if v.P != nil {
+		stkLen := len(er.ctxStack)
+		val := er.ctxStack[stkLen-1]
+		intNum, isNull, err := expression.GetIntFromConstant(er.ctx, val)
+		str = "?"
+		if err == nil {
+			if isNull {
+				return
+			}
+			pos = intNum
+			er.ctxStack = er.ctxStack[:stkLen-1]
+		}
+		er.err = err
+	}
+	if er.err == nil && pos > 0 && pos <= er.schema.Len() {
+		er.ctxStack = append(er.ctxStack, er.schema.Columns[pos-1])
 	} else {
-		er.err = ErrUnknownColumn.GenWithStackByArgs(strconv.Itoa(v.N), clauseMsg[er.b.curClause])
+		er.err = ErrUnknownColumn.GenWithStackByArgs(str, clauseMsg[er.b.curClause])
 	}
 }