Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Oct 14, 2019
Merged
Show file tree
Hide file tree
Changes from 6 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 executor/prepared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
51 changes: 51 additions & 0 deletions executor/prepared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)'")
}
}
8 changes: 5 additions & 3 deletions expression/simple_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
72 changes: 72 additions & 0 deletions expression/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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-20191010030522-6e4f15d4819b
winoros marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -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-20191009170203-06d7bd2c5f4f // 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
Expand Down
17 changes: 9 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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-20191010030522-6e4f15d4819b h1:1rJkZLJt+p5Cg58zBMAB0anvOFlxsDiLPTBOXutSJ5U=
github.com/pingcap/parser v0.0.0-20191010030522-6e4f15d4819b/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=
Expand Down Expand Up @@ -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-20191009170203-06d7bd2c5f4f h1:hjzMYz/7Ea1mNKfOnFOfktR0mlA5jqhvywClCMHM/qw=
golang.org/x/sys v0.0.0-20191009170203-06d7bd2c5f4f/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=
Expand Down
14 changes: 14 additions & 0 deletions planner/core/cacheable_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions planner/core/cacheable_checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 2 additions & 0 deletions planner/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (

codeWrongUsage = mysql.ErrWrongUsage
codeAmbiguous = mysql.ErrNonUniq
codeUnknown = mysql.ErrUnknown
codeUnknownColumn = mysql.ErrBadField
codeUnknownTable = mysql.ErrUnknownTable
codeWrongArguments = mysql.ErrWrongArguments
Expand Down Expand Up @@ -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])
Expand Down
31 changes: 23 additions & 8 deletions planner/core/expression_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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])
}
}

Expand Down
Loading