*: add buildin function CONV #2390
Conversation
| @@ -190,8 +192,68 @@ func builtinRound(args []types.Datum, ctx context.Context) (d types.Datum, err e | |||
|
|
|||
| // See http://dev.mysql.com/doc/refman/5.7/en/mathematical-functions.html#function_conv | |||
| func builtinConv(args []types.Datum, ctx context.Context) (d types.Datum, err error) { | |||
| //TODO implement | |||
| return d, errors.New("Function unimplement") | |||
| // TODO: Implement it. | |||
| @@ -98,3 +100,38 @@ func calculateSum(sc *variable.StatementContext, sum, v types.Datum) (data types | |||
| return data, errors.Errorf("invalid value %v for aggregate", sum.Kind()) | |||
| } | |||
| } | |||
|
|
|||
| // getValidPrefix gets a prefix of string which can parsed to integer. | |||
| func getValidPrefix(s string, base int64) string { | |||
There was a problem hiding this comment.
This function can be write more elegant, Rest LGTM
| fromBase = -fromBase | ||
| } | ||
| if toBase < 0 { | ||
| signed = true |
There was a problem hiding this comment.
"If from_base is a negative number, N is regarded as a signed number. Otherwise, N is treated as unsigned. "
So singed = fromBase < 0?
There was a problem hiding this comment.
the docs seems have a error , I test in mysql, just toBase affect signed.
There was a problem hiding this comment.
Please refer to mysql source code: https://github.com/mysql/mysql-server/blob/5.7/sql/item_strfunc.cc#L3959
| {[]interface{}{"6E", 18, 8}, "172"}, | ||
| {[]interface{}{"-17", 10, -18}, "-H"}, | ||
| {[]interface{}{nil, 10, 10}, nil}, | ||
| {[]interface{}{"+18aZ", 7, 36}, 1}, |
There was a problem hiding this comment.
Can we handle this case: "SELECT CONV(10+'10'+'10'+X'0a',10,10);" ?
There was a problem hiding this comment.
yes we can, constant folding will handle it.
There was a problem hiding this comment.
10+'10'+'10'+X'0a' will handle firstly by opfunction, add in here will no work.
| @@ -98,3 +100,42 @@ func calculateSum(sc *variable.StatementContext, sum, v types.Datum) (data types | |||
| return data, errors.Errorf("invalid value %v for aggregate", sum.Kind()) | |||
| } | |||
| } | |||
|
|
|||
| // getValidPrefix gets a prefix of string which can parsed to a number with base. the minimun base is 2 and the maximum is 36. | |||
| func getValidPrefix(s string, base int64) string { | |||
There was a problem hiding this comment.
Can you add some unit test cases for this function?
|
LGTM |
| return d, nil | ||
| } | ||
| } | ||
| n, err = args[0].ToString() |
There was a problem hiding this comment.
Use := instead of =, and remove line 196.
The same as line 213, line 217 and line 233.
| if err != nil { | ||
| return d, errors.Trace(types.ErrOverflow) | ||
| } | ||
| //Ref https://github.com/mysql/mysql-server/blob/5.7/strings/ctype-simple.c#L598 |
There was a problem hiding this comment.
Add a space after the slash. And using See instead of Ref is better.
The same as line258.
| fromBase int64 | ||
| signed bool | ||
| negative bool | ||
| touval bool |
There was a problem hiding this comment.
What's the meaning of touval?
There was a problem hiding this comment.
it means when tobase < 0 , we let n to unsigned ignore the sign. example -15 to 15, 15 to 15. if tobase > 0, n will be forced conversion.
| {[]interface{}{"-17", 10, -18}, "-H"}, | ||
| {[]interface{}{"-17", 10, 18}, "2D3FGB0B9CG4BD1H"}, | ||
| {[]interface{}{nil, 10, 10}, nil}, | ||
| {[]interface{}{"+18aZ", 7, 36}, 1}, |
There was a problem hiding this comment.
Add a test with a negative arg[1].
|
PTAL @zimulala @tiancaiamao |
|
LGTM |
| if fromBase > 36 || fromBase < 2 || toBase > 36 || toBase < 2 { | ||
| return d, nil | ||
| } | ||
| n = getValidPrefix(n, fromBase) |
There was a problem hiding this comment.
We should trim 'n' before calling getValidPrefix.
| if fromBase > 36 || fromBase < 2 || toBase > 36 || toBase < 2 { | ||
| return d, nil | ||
| } | ||
| n = getValidPrefix(strings.TrimSpace(n), fromBase) |
There was a problem hiding this comment.
Add a test about args[0] with spaces.
For #2342