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

*: improve syntax error code & message compatibility #9103

Merged
merged 7 commits into from
Jan 23, 2019
Merged
Show file tree
Hide file tree
Changes from 5 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
5 changes: 3 additions & 2 deletions executor/prepared.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/variable"
driver "github.com/pingcap/tidb/types/parser_driver"
"github.com/pingcap/tidb/util"
"github.com/pingcap/tidb/util/chunk"
"github.com/pingcap/tidb/util/sqlexec"
)
Expand Down Expand Up @@ -117,11 +118,11 @@ func (e *PrepareExec) Next(ctx context.Context, req *chunk.RecordBatch) error {
var warns []error
stmts, warns, err = p.Parse(e.sqlText, charset, collation)
for _, warn := range warns {
e.ctx.GetSessionVars().StmtCtx.AppendWarning(warn)
e.ctx.GetSessionVars().StmtCtx.AppendWarning(util.SyntaxWarn(warn))
}
}
if err != nil {
return errors.Trace(err)
return util.SyntaxError(err)
}
if len(stmts) != 1 {
return ErrPrepareMulti
Expand Down
6 changes: 2 additions & 4 deletions executor/seqtest/prepared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (
"math"

. "github.com/pingcap/check"
"github.com/pingcap/errors"
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/executor"
"github.com/pingcap/tidb/metrics"
plannercore "github.com/pingcap/tidb/planner/core"
Expand Down Expand Up @@ -71,7 +69,7 @@ func (s *seqTestSuite) TestPrepared(c *C) {

// incorrect SQLs in prepare. issue #3738, SQL in prepare stmt is parsed in DoPrepare.
_, err = tk.Exec(`prepare p from "delete from t where a = 7 or 1=1/*' and b = 'p'";`)
c.Assert(terror.ErrorEqual(err, errors.New(`[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '/*' and b = 'p'' at line 1`)), IsTrue, Commentf("err %v", err))
c.Assert(err.Error(), Equals, `[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use near '/*' and b = 'p'' at line 1`)

// The `stmt_test5` should not be found.
_, err = tk.Exec(`set @a = 1; execute stmt_test_5 using @a;`)
Expand Down Expand Up @@ -218,7 +216,7 @@ func (s *seqTestSuite) TestPrepared(c *C) {
tk.MustExec("create table prepare1 (a decimal(1))")
tk.MustExec("insert into prepare1 values(1);")
_, err = tk.Exec("prepare stmt FROM @sql1")
c.Assert(err.Error(), Equals, "line 1 column 4 near \"NULL\" (total length 4)")
c.Assert(err.Error(), Equals, "[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 4 near \"NULL\" ")
tk.MustExec("SET @sql = 'update prepare1 set a=5 where a=?';")
_, err = tk.Exec("prepare stmt FROM @sql")
c.Assert(err, IsNil)
Expand Down
2 changes: 1 addition & 1 deletion expression/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3737,7 +3737,7 @@ func (s *testIntegrationSuite) TestUnknowHintIgnore(c *C) {
tk.MustExec("USE test")
tk.MustExec("create table t(a int)")
tk.MustQuery("select /*+ unknown_hint(c1)*/ 1").Check(testkit.Rows("1"))
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1105 line 1 column 29 near \"select /*+ unknown_hint(c1)*/ 1\" (total length 31)"))
tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1064 You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 29 near \"unknown_hint(c1)*/ 1\" "))
_, err := tk.Exec("select 1 from /*+ test1() */ t")
c.Assert(err, NotNil)
}
Expand Down
9 changes: 5 additions & 4 deletions expression/simple_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/types/parser_driver"
"github.com/pingcap/tidb/util"
)

type simpleRewriter struct {
Expand All @@ -39,10 +40,10 @@ func ParseSimpleExprWithTableInfo(ctx sessionctx.Context, exprStr string, tableI
exprStr = "select " + exprStr
stmts, warns, err := parser.New().Parse(exprStr, "", "")
for _, warn := range warns {
ctx.GetSessionVars().StmtCtx.AppendWarning(warn)
ctx.GetSessionVars().StmtCtx.AppendWarning(util.SyntaxWarn(warn))
}
if err != nil {
return nil, err
return nil, util.SyntaxError(err)
}
expr := stmts[0].(*ast.SelectStmt).Fields.Fields[0].Expr
return RewriteSimpleExprWithTableInfo(ctx, tableInfo, expr)
Expand Down Expand Up @@ -77,10 +78,10 @@ func ParseSimpleExprsWithSchema(ctx sessionctx.Context, exprStr string, schema *
exprStr = "select " + exprStr
stmts, warns, err := parser.New().Parse(exprStr, "", "")
for _, warn := range warns {
ctx.GetSessionVars().StmtCtx.AppendWarning(warn)
ctx.GetSessionVars().StmtCtx.AppendWarning(util.SyntaxWarn(warn))
}
if err != nil {
return nil, err
return nil, util.SyntaxWarn(err)
}
fields := stmts[0].(*ast.SelectStmt).Fields.Fields
exprs := make([]Expression, 0, len(fields))
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ require (
github.com/pingcap/gofail v0.0.0-20181217135706-6a951c1e42c3
github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e
github.com/pingcap/kvproto v0.0.0-20190110035000-d4fe6b336379
github.com/pingcap/parser v0.0.0-20190117104731-c02a7ccd3d6b
github.com/pingcap/parser v0.0.0-20190121074657-4b899f19591e
github.com/pingcap/pd v2.1.0-rc.4+incompatible
github.com/pingcap/tidb-tools v2.1.3-0.20190104033906-883b07a04a73+incompatible
github.com/pingcap/tipb v0.0.0-20181012112600-11e33c750323
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e h1:P73/4dPCL96rG
github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw=
github.com/pingcap/kvproto v0.0.0-20190110035000-d4fe6b336379 h1:l4KInBOtxjbgQLjCFHzX66vZgNzsH4a+RiuVZGrO0xk=
github.com/pingcap/kvproto v0.0.0-20190110035000-d4fe6b336379/go.mod h1:QMdbTAXCHzzygQzqcG9uVUgU2fKeSN1GmfMiykdSzzY=
github.com/pingcap/parser v0.0.0-20190117104731-c02a7ccd3d6b h1:Z2qM+gP9BkJA1lg6Kai7Uy9kHT0EXF9U8BqYzP4Jcj4=
github.com/pingcap/parser v0.0.0-20190117104731-c02a7ccd3d6b/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
github.com/pingcap/parser v0.0.0-20190121074657-4b899f19591e h1:UxQ0Fj4NDpQ0SSZ0Z76je8OPSDlbRxMfXTbyPrFs6c4=
github.com/pingcap/parser v0.0.0-20190121074657-4b899f19591e/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA=
github.com/pingcap/pd v2.1.0-rc.4+incompatible h1:/buwGk04aHO5odk/+O8ZOXGs4qkUjYTJ2UpCJXna8NE=
github.com/pingcap/pd v2.1.0-rc.4+incompatible/go.mod h1:nD3+EoYes4+aNNODO99ES59V83MZSI+dFbhyr667a0E=
github.com/pingcap/tidb-tools v2.1.3-0.20190104033906-883b07a04a73+incompatible h1:Ba48wwPwPq5hd1kkQpgua49dqB5cthC2zXVo7fUUDec=
Expand Down
4 changes: 2 additions & 2 deletions session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ func (s *session) execute(ctx context.Context, sql string) (recordSets []sqlexec
if err != nil {
s.rollbackOnError(ctx)
log.Warnf("con:%d parse error:\n%v\n%s", connID, err, sql)
return nil, errors.Trace(err)
return nil, util.SyntaxError(err)
}
label := s.getSQLLabel()
metrics.SessionExecuteParseDuration.WithLabelValues(label).Observe(time.Since(startTS).Seconds())
Expand Down Expand Up @@ -967,7 +967,7 @@ func (s *session) execute(ctx context.Context, sql string) (recordSets []sqlexec
}

for _, warn := range warns {
s.sessionVars.StmtCtx.AppendWarning(warn)
s.sessionVars.StmtCtx.AppendWarning(util.SyntaxWarn(warn))
}
return recordSets, nil
}
Expand Down
3 changes: 2 additions & 1 deletion table/tables/gen_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/pingcap/parser/ast"
"github.com/pingcap/parser/charset"
"github.com/pingcap/parser/model"
"github.com/pingcap/tidb/util"
)

// nameResolver is the visitor to resolve table name and column name.
Expand Down Expand Up @@ -65,7 +66,7 @@ func parseExpression(expr string) (node ast.ExprNode, err error) {
if err == nil {
node = stmts[0].(*ast.SelectStmt).Fields.Fields[0].Expr
}
return node, errors.Trace(err)
return node, util.SyntaxError(err)
}

// SimpleResolveName resolves all column names in the expression node.
Expand Down
23 changes: 23 additions & 0 deletions util/misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"time"

"github.com/pingcap/errors"
"github.com/pingcap/parser"
log "github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -95,3 +96,25 @@ func CompatibleParseGCTime(value string) (time.Time, error) {
}
return t, err
}

const (
// SyntaxErrorPrefix is the common prefix for SQL syntax error in TiDB.
lysu marked this conversation as resolved.
Show resolved Hide resolved
SyntaxErrorPrefix = "You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use"
)

// SyntaxError converts parser error to TiDB's syntax error.
func SyntaxError(err error) error {
if err == nil {
return nil
}
log.Errorf("%+v", err)
return parser.ErrParse.GenWithStackByArgs(SyntaxErrorPrefix, err.Error())
}

// SyntaxWarn converts parser warn to TiDB's syntax warn.
func SyntaxWarn(err error) error {
if err == nil {
return nil
}
return parser.ErrParse.GenWithStackByArgs(SyntaxErrorPrefix, err.Error())
}