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

support function rpad #2270

Merged
merged 7 commits into from
Dec 19, 2016
Merged

support function rpad #2270

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift click to select a range
cac3530
support function rpad
silentred Dec 18, 2016
8daa1c2
add typeinferer info; add test case for long pad string
silentred Dec 19, 2016
3d47547
distinguish expecting values of null and empty string
silentred Dec 19, 2016
3a45079
use string compacting for rpad
silentred Dec 19, 2016
94b357d
use strings.Repeat for rpad
silentred Dec 19, 2016
f2161a0
add test case for typeinferer
silentred Dec 19, 2016
c6265c1
Merge branch 'master' into feature-rpad
tiancaiamao Dec 19, 2016
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
1 change: 1 addition & 0 deletions ast/functions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 145,7 @@ const (
Ucase = "ucase"
Hex = "hex"
Unhex = "unhex"
Rpad = "rpad"

// information functions
ConnectionID = "connection_id"
Expand Down
1 change: 1 addition & 0 deletions evaluator/builtin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 117,7 @@ var Funcs = map[string]Func{
ast.Ucase: {builtinUpper, 1, 1},
ast.Hex: {builtinHex, 1, 1},
ast.Unhex: {builtinUnHex, 1, 1},
ast.Rpad: {builtinRpad, 3, 3},

// information functions
ast.ConnectionID: {builtinConnectionID, 0, 0},
Expand Down
46 changes: 46 additions & 0 deletions evaluator/builtin_string.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,3 603,49 @@ func trimRight(str, remstr string) string {
str = x
}
}

// See https://dev.mysql.com/doc/refman/5.7/en/string-functions.html#function_rpad
func builtinRpad(args []types.Datum, ctx context.Context) (d types.Datum, err error) {
// RPAD(str,len,padstr)
// args[0] string, args[1] int, args[2] string
str, err := args[0].ToString()
if err != nil {
return d, errors.Trace(err)
}
length, err := args[1].ToInt64(ctx.GetSessionVars().StmtCtx)
if err != nil {
return d, errors.Trace(err)
}
l := int(length)

padStr, err := args[2].ToString()
if err != nil {
return d, errors.Trace(err)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, the following logic too much complex than it should be, and the loop is hard to read.
Here is a simpler example:

func test(str string, length int, pad string) {
	for len(str) < length {
		str = str   pad
	}
	return str[:length]
}

just check pad != "" before this logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, it is simpler indeed. I worried that str = str pad would allocate memory every time, so I chose pre-allocated []byte.

if len(str) >= l && l >= 0 {
d.SetString(str[:l])
return d, nil
}

if l < 0 || len(padStr) == 0 {
d.SetNull()
return d, nil
}

tailLen := l - len(str)
padStrBytes := []byte(padStr)
Copy link
Member

Choose a reason for hiding this comment

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

How about

repeatCount := tailLen/len(padStr)   1
dest := str   strings.Repeat(padStr, repeatCount)
dest = dest[:l]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks better. I will change.

tail := make([]byte, 0, tailLen)
for tailLen > len(padStr) {
tail = append(tail, padStrBytes...)
tailLen -= len(padStr)
}
if tailLen > 0 {
tail = append(tail, padStrBytes[:tailLen]...)
}

dest := str string(tail)
d.SetString(dest)

return d, nil
}
31 changes: 31 additions & 0 deletions evaluator/builtin_string_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,3 696,34 @@ func (s *testEvaluatorSuite) TestUnhexFunc(c *C) {

}
}

func (s *testEvaluatorSuite) TestRpad(c *C) {
tests := []struct {
str string
len int64
padStr string
expect interface{}
}{
{"hi", 5, "?", "hi???"},
{"hi", 1, "?", "h"},
{"hi", 0, "?", ""},
{"hi", -1, "?", nil},
{"hi", 1, "", "h"},
{"hi", 5, "", nil},
{"hi", 5, "ab", "hiaba"},
{"hi", 6, "ab", "hiabab"},
Copy link
Member

Choose a reason for hiding this comment

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

The test can't distinguish null and empty string.
rpad("hi", 0, "?") is empty string, not NULL.

}
for _, test := range tests {
str := types.NewStringDatum(test.str)
length := types.NewIntDatum(test.len)
padStr := types.NewStringDatum(test.padStr)
result, err := builtinRpad([]types.Datum{str, length, padStr}, s.ctx)
c.Assert(err, IsNil)
if test.expect == nil {
c.Assert(result.Kind(), Equals, types.KindNull)
} else {
expect, _ := test.expect.(string)
c.Assert(result.GetString(), Equals, expect)
}
}
}
1 change: 1 addition & 0 deletions parser/misc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 479,7 @@ var tokenMap = map[string]int{
"ACTION": action,
"PARTITION": partition,
"PARTITIONS": partitions,
"RPAD": rpad,
}

func isTokenIdentifier(s string, buf *bytes.Buffer) int {
Expand Down
8 changes: 8 additions & 0 deletions parser/parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 286,7 @@ import (
statsPersistent "STATS_PERSISTENT"
getLock "GET_LOCK"
releaseLock "RELEASE_LOCK"
rpad "RPAD"

/* the following tokens belong to UnReservedKeyword*/
action "ACTION"
Expand Down Expand Up @@ -2970,6 2971,13 @@ FunctionCallNonKeyword:
{
$$ = &ast.FuncCallExpr{FnName: model.NewCIStr($1), Args: []ast.ExprNode{$3.(ast.ExprNode)}}
}
| "RPAD" '(' Expression ',' Expression ',' Expression ')'
{
$$ = &ast.FuncCallExpr{
FnName: model.NewCIStr($1),
Args: []ast.ExprNode{$3.(ast.ExprNode), $5.(ast.ExprNode), $7.(ast.ExprNode)},
}
}

DateArithOpt:
"DATE_ADD"
Expand Down
2 changes: 1 addition & 1 deletion plan/typeinferer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 311,7 @@ func (v *typeInferrer) handleFuncCallExpr(x *ast.FuncCallExpr) {
case "dayname", "version", "database", "user", "current_user", "schema",
"concat", "concat_ws", "left", "lcase", "lower", "repeat",
"replace", "ucase", "upper", "convert", "substring",
"substring_index", "trim", "ltrim", "rtrim", "reverse", "hex", "unhex", "date_format":
"substring_index", "trim", "ltrim", "rtrim", "reverse", "hex", "unhex", "date_format", "rpad":
tp = types.NewFieldType(mysql.TypeVarString)
chs = v.defaultCharset
case "strcmp", "isnull":
Expand Down