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.

Sign up for GitHub

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

parser: support sql_mode 'IGNORE SPACE' #5106

Merged
merged 18 commits into from
Dec 7, 2017
Merged

parser: support sql_mode 'IGNORE SPACE' #5106

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8a4bbe8
parser: support sql_mode 'IGNORE_SPACE'
spongedu Nov 14, 2017
694b8c0
Merge remote-tracking branch 'upstream/master' into mode_ignore_space
spongedu Nov 14, 2017
8439169
Fix regression
spongedu Nov 15, 2017
2730b3b
revert #5073 related codes according to #5104
spongedu Nov 15, 2017
a04a3b2
Merge remote-tracking branch 'upstream/master' into mode_ignore_space
spongedu Nov 18, 2017
6276a97
Code format refine
spongedu Nov 20, 2017
a941c25
Merge remote-tracking branch 'upstream/master' into mode_ignore_space
spongedu Nov 22, 2017
fdb6110
Merge branch 'master' into mode_ignore_space
spongedu Nov 23, 2017
e61e76a
1, Bug Fix: 'bit_xxx' and 'now' are not correctly recognized in 'IGNO…
spongedu Nov 24, 2017
2d53514
Merge branch 'master' into mode_ignore_space
spongedu Nov 27, 2017
8380384
Format refine
spongedu Nov 27, 2017
6ef56f7
Merge branch 'master' into mode_ignore_space
spongedu Nov 29, 2017
dd69562
Merge branch 'master' into mode_ignore_space
spongedu Dec 1, 2017
763145c
Merge branch 'master' into mode_ignore_space
zimulala Dec 6, 2017
8846cc7
Merge branch 'master' into mode_ignore_space
zimulala Dec 6, 2017
87b62fd
Merge branch 'master' into mode_ignore_space
spongedu Dec 6, 2017
5b75b17
Merge remote-tracking branch 'upstream/master' into mode_ignore_space
spongedu Dec 6, 2017
76b4f26
Fix case fail
spongedu Dec 6, 2017
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: 5 additions & 0 deletions mysql/const.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,11 @@ func (m SQLMode) HasNoBackslashEscapesMode() bool {
return m&ModeNoBackslashEscapes == ModeNoBackslashEscapes
}

// HasIgnoreSpaceMode detects if 'IGNORE_SPACE' mode is set in SQLMode
func (m SQLMode) HasIgnoreSpaceMode() bool {
return m&ModeIgnoreSpace == ModeIgnoreSpace
}

// consts for sql modes.
const (
ModeNone SQLMode = 0
Expand Down
20 changes: 20 additions & 0 deletions mysql/const_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,26 @@ func (s *testMySQLConstSuite) TestHighNotPrecedenceMode(c *C) {
r.Check(testkit.Rows("1"))
}

func (s *testMySQLConstSuite) TestIgnoreSpaceMode(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("set sql_mode=''")
tk.MustExec("CREATE TABLE COUNT (a bigint);")
tk.MustExec("DROP TABLE COUNT;")
tk.MustExec("CREATE TABLE `COUNT` (a bigint);")
tk.MustExec("DROP TABLE COUNT;")
_, err := tk.Exec("CREATE TABLE COUNT(a bigint);")
c.Assert(err, NotNil)

tk.MustExec("set sql_mode='IGNORE_SPACE'")
_, err = tk.Exec("CREATE TABLE COUNT (a bigint);")
c.Assert(err, NotNil)
tk.MustExec("CREATE TABLE `COUNT` (a bigint);")
tk.MustExec("DROP TABLE COUNT;")
_, err = tk.Exec("CREATE TABLE COUNT(a bigint);")
c.Assert(err, NotNil)
}

func (s *testMySQLConstSuite) TestNoBackslashEscapesMode(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("set sql_mode=''")
Expand Down
46 changes: 45 additions & 1 deletion parser/lexer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,41 @@ import (

var _ = yyLexer(&Scanner{})

var ignoreSpaceFuncMap = map[string]int{
Copy link
Member

Choose a reason for hiding this comment

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

How do we obtain tis map ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zz-jason I've rewrite this part. PTAL

"adddate": builtinAddDate,
"bit_and": builtinBitAnd,
"bit_or": builtinBitOr,
"bit_xor": builtinBitXor,
"cast": builtinCast,
"count": builtinCount,
"curdate": builtinCurDate,
"curtime": builtinCurTime,
"date_add": builtinDateAdd,
"date_sub": builtinDateSub,
"extract": builtinExtract,
"group_concat": builtinGroupConcat,
"max": builtinMax,
"mid": builtinSubstring,
"min": builtinMin,
"now": builtinNow,
"position": builtinPosition,
"session_user": builtinUser,
"std": builtinStd,
"stddev": builtinStddev,
"stddev_pop": builtinStddevPop,
"stddev_samp": builtinStddevSamp,
"subdate": builtinSubDate,
"substr": builtinSubstring,
"substring": builtinSubstring,
"sum": builtinSum,
"sysdate": builtinSysDate,
"system_user": builtinUser,
"trim": builtinTrim,
"variance": builtinVariance,
"var_Pop": builtinVarPop,
"var_samp": builtinVarSamp,
}

// Pos represents the position of a token.
type Pos struct {
Line int
Expand Down Expand Up @@ -435,7 +470,16 @@ func scanIdentifier(s *Scanner) (int, Pos, string) {
pos := s.r.pos()
s.r.inc()
s.r.incAsLongAs(isIdentChar)
return identifier, pos, s.r.data(&pos)
lit := s.r.data(&pos)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's quite performance sensitive here.
strings.ToLower(lit) would cause allocation, and map access is not that fast.
I think you can switch the order, check IgnoreSpaceMode first, try the best not to hurt the performance in most cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tiancaiamao ok, I'll take a look. thx :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tiancaiamao I've refine this place. PTAL

if t, ok := ignoreSpaceFuncMap[strings.ToLower(lit)]; ok {
if s.sqlMode.HasIgnoreSpaceMode() {
s.skipWhitespace()
}
if s.r.peek() == '(' {
return t, pos, lit
}
}
return identifier, pos, lit
}

var (
Expand Down
94 changes: 68 additions & 26 deletions parser/parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,36 @@ import (
tidbSMJ "TIDB_SMJ"
tidbINLJ "TIDB_INLJ"

builtinAddDate
builtinBitAnd
builtinBitOr
builtinBitXor
builtinCast
builtinCount
builtinCurDate
builtinCurTime
builtinDateAdd
builtinDateSub
builtinExtract
builtinGroupConcat
builtinMax
builtinMin
builtinNow
builtinPosition
builtinStd
builtinStddev
builtinStddevPop
builtinStddevSamp
builtinSubDate
builtinSubstring
builtinSum
builtinSysDate
builtinTrim
builtinUser
builtinVariance
builtinVarPop
builtinVarSamp

%token <item>

/*yy:token "1.%d" */ floatLit "floating-point literal"
Expand Down Expand Up @@ -1456,7 +1486,7 @@ NowSymOptionFraction:
* TODO: Process other three keywords
*/
NowSymFunc:
"CURRENT_TIMESTAMP" | "LOCALTIME" | "LOCALTIMESTAMP" | "NOW"
"CURRENT_TIMESTAMP" | "LOCALTIME" | "LOCALTIMESTAMP" | builtinNow
NowSym:
"CURRENT_TIMESTAMP" | "LOCALTIME" | "LOCALTIMESTAMP"

Expand Down Expand Up @@ -2829,7 +2859,7 @@ SimpleExpr:
FunctionType: ast.CastBinaryOperator,
}
}
| "CAST" '(' Expression "AS" CastType ')'
| builtinCast '(' Expression "AS" CastType ')'
{
/* See https://dev.mysql.com/doc/refman/5.7/en/cast-functions.html#function_cast */
tp := $5.(*types.FieldType)
Expand Down Expand Up @@ -2953,7 +2983,7 @@ FunctionNameConflict:
| "MICROSECOND"
| "MINUTE"
| "MONTH"
| "NOW"
| builtinNow
| "QUARTER"
| "REPEAT"
| "REPLACE"
Expand Down Expand Up @@ -2989,10 +3019,18 @@ FunctionCallKeyword:
{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1), Args: $3.([]ast.ExprNode)}
}
| builtinUser '(' ExpressionListOpt ')'
Copy link
Member

Choose a reason for hiding this comment

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

keep the code indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zz-jason ok

{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1), Args: $3.([]ast.ExprNode)}
}
| FunctionNameOptionalBraces OptionalBraces
{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1)}
}
| builtinCurDate '(' ')'
Copy link
Member

Choose a reason for hiding this comment

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

keep code indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1)}
}
| FunctionNameDatetimePrecision FuncDatetimePrec
{
args := []ast.ExprNode{}
Expand Down Expand Up @@ -3048,7 +3086,11 @@ FunctionCallKeyword:
}

FunctionCallNonKeyword:
"CURTIME" '(' FuncDatetimePrecListOpt ')'
builtinCurTime '(' FuncDatetimePrecListOpt ')'
{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1), Args: $3.([]ast.ExprNode)}
}
| builtinSysDate '(' FuncDatetimePrecListOpt ')'
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1), Args: $3.([]ast.ExprNode)}
}
Expand Down Expand Up @@ -3085,7 +3127,7 @@ FunctionCallNonKeyword:
},
}
}
| "EXTRACT" '(' TimeUnit "FROM" Expression ')'
| builtinExtract '(' TimeUnit "FROM" Expression ')'
{
timeUnit := ast.NewValueExpr($3)
$$ = &ast.FuncCallExpr{
Expand All @@ -3100,32 +3142,32 @@ FunctionCallNonKeyword:
Args: []ast.ExprNode{ast.NewValueExpr($3), $5},
}
}
| "POSITION" '(' BitExpr "IN" Expression ')'
| builtinPosition '(' BitExpr "IN" Expression ')'
{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1), Args: []ast.ExprNode{$3, $5}}
}
| "SUBSTRING" '(' Expression ',' Expression ')'
| builtinSubstring '(' Expression ',' Expression ')'
{
$$ = &ast.FuncCallExpr{
FnName: model.NewCIStr($1),
Args: []ast.ExprNode{$3, $5},
}
}
| "SUBSTRING" '(' Expression "FROM" Expression ')'
| builtinSubstring '(' Expression "FROM" Expression ')'
{
$$ = &ast.FuncCallExpr{
FnName: model.NewCIStr($1),
Args: []ast.ExprNode{$3, $5},
}
}
| "SUBSTRING" '(' Expression ',' Expression ',' Expression ')'
| builtinSubstring '(' Expression ',' Expression ',' Expression ')'
{
$$ = &ast.FuncCallExpr{
FnName: model.NewCIStr($1),
Args: []ast.ExprNode{$3, $5, $7},
}
}
| "SUBSTRING" '(' Expression "FROM" Expression "FOR" Expression ')'
| builtinSubstring '(' Expression "FROM" Expression "FOR" Expression ')'
{
$$ = &ast.FuncCallExpr{
FnName: model.NewCIStr($1),
Expand All @@ -3146,21 +3188,21 @@ FunctionCallNonKeyword:
Args: []ast.ExprNode{ast.NewValueExpr($3), $5, $7},
}
}
| "TRIM" '(' Expression ')'
| builtinTrim '(' Expression ')'
{
$$ = &ast.FuncCallExpr{
FnName: model.NewCIStr($1),
Args: []ast.ExprNode{$3},
}
}
| "TRIM" '(' Expression "FROM" Expression ')'
| builtinTrim '(' Expression "FROM" Expression ')'
{
$$ = &ast.FuncCallExpr{
FnName: model.NewCIStr($1),
Args: []ast.ExprNode{$5, $3},
}
}
| "TRIM" '(' TrimDirection "FROM" Expression ')'
| builtinTrim '(' TrimDirection "FROM" Expression ')'
{
nilVal := ast.NewValueExpr(nil)
direction := ast.NewValueExpr(int($3.(ast.TrimDirectionType)))
Expand All @@ -3169,7 +3211,7 @@ FunctionCallNonKeyword:
Args: []ast.ExprNode{$5, nilVal, direction},
}
}
| "TRIM" '(' TrimDirection Expression "FROM" Expression ')'
| builtinTrim '(' TrimDirection Expression "FROM" Expression ')'
{
direction := ast.NewValueExpr(int($3.(ast.TrimDirectionType)))
$$ = &ast.FuncCallExpr{
Expand Down Expand Up @@ -3198,13 +3240,13 @@ GetFormatSelector:


FunctionNameDateArith:
"DATE_ADD"
| "DATE_SUB"
builtinDateAdd
| builtinDateSub


FunctionNameDateArithMultiForms:
"ADDDATE"
| "SUBDATE"
builtinAddDate
| builtinSubDate


TrimDirection:
Expand Down Expand Up @@ -3238,36 +3280,36 @@ SumExpr:
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: []ast.ExprNode{$3}}
}
| "COUNT" '(' DistinctKwd ExpressionList ')'
| builtinCount '(' DistinctKwd ExpressionList ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: $4.([]ast.ExprNode), Distinct: true}
}
| "COUNT" '(' "ALL" Expression ')'
| builtinCount '(' "ALL" Expression ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: []ast.ExprNode{$4}}
}
| "COUNT" '(' Expression ')'
| builtinCount '(' Expression ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: []ast.ExprNode{$3}}
}
| "COUNT" '(' '*' ')'
| builtinCount '(' '*' ')'
{
args := []ast.ExprNode{ast.NewValueExpr(1)}
$$ = &ast.AggregateFuncExpr{F: $1, Args: args}
}
| "GROUP_CONCAT" '(' BuggyDefaultFalseDistinctOpt ExpressionList ')'
| builtinGroupConcat '(' BuggyDefaultFalseDistinctOpt ExpressionList ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: $4.([]ast.ExprNode), Distinct: $3.(bool)}
}
| "MAX" '(' BuggyDefaultFalseDistinctOpt Expression ')'
| builtinMax '(' BuggyDefaultFalseDistinctOpt Expression ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: []ast.ExprNode{$4}, Distinct: $3.(bool)}
}
| "MIN" '(' BuggyDefaultFalseDistinctOpt Expression ')'
| builtinMin '(' BuggyDefaultFalseDistinctOpt Expression ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: []ast.ExprNode{$4}, Distinct: $3.(bool)}
}
| "SUM" '(' BuggyDefaultFalseDistinctOpt Expression ')'
| builtinSum '(' BuggyDefaultFalseDistinctOpt Expression ')'
{
$$ = &ast.AggregateFuncExpr{F: $1, Args: []ast.ExprNode{$4}, Distinct: $3.(bool)}
}
Expand Down
2 changes: 1 addition & 1 deletion parser/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ func (s *testParserSuite) TestBuiltin(c *C) {

// for date, day, weekday
{"SELECT CURRENT_DATE, CURRENT_DATE(), CURDATE()", true},
{"SELECT CURRENT_DATE, CURRENT_DATE(), CURDATE(1)", true},
{"SELECT CURRENT_DATE, CURRENT_DATE(), CURDATE(1)", false},
Copy link
Member

Choose a reason for hiding this comment

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

Why this is false case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shenli it seems that MySQL doesn't support CURDATE with one parameter. see: https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_curdate

{"SELECT DATEDIFF('2003-12-31', '2003-12-30');", true},
{"SELECT DATE('2003-12-31 01:02:03');", true},
{"SELECT DATE();", true},
Expand Down