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

Jump to bottom

expression: rewrite builtin function: INSERT #4414

Merged
merged 4 commits into from Sep 7, 2017
Merged

expression: rewrite builtin function: INSERT #4414

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c823377
expression: rewrite builtin function: INSERT
zz-jason Sep 4, 2017
b46927e
address comment
zz-jason Sep 5, 2017
d0cdf95
Merge branch 'master' into zz-jason/expression/string/insert
zz-jason Sep 6, 2017
b44f7ba
Merge branch 'master' of https://github.com/pingcap/tidb into zz-jaso…
zz-jason Sep 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion expression/builtin.go
View file
Open in desktop
Expand Up @@ -803,7 +803,7 @@ var funcs = map[string]functionClass{
ast.Field: &fieldFunctionClass{baseFunctionClass{ast.Field, 2, -1}},
ast.Format: &formatFunctionClass{baseFunctionClass{ast.Format, 2, 3}},
ast.FromBase64: &fromBase64FunctionClass{baseFunctionClass{ast.FromBase64, 1, 1}},
ast.InsertFunc: &insertFuncFunctionClass{baseFunctionClass{ast.InsertFunc, 4, 4}},
ast.InsertFunc: &insertFunctionClass{baseFunctionClass{ast.InsertFunc, 4, 4}},
ast.Instr: &instrFunctionClass{baseFunctionClass{ast.Instr, 2, 2}},
ast.Lcase: &lowerFunctionClass{baseFunctionClass{ast.Lcase, 1, 1}},
ast.Left: &leftFunctionClass{baseFunctionClass{ast.Left, 2, 2}},
Expand Down
116 changes: 76 additions & 40 deletions expression/builtin_string.go
View file
Open in desktop
Expand Up @@ -78,7 +78,7 @@ var (
_ functionClass = &formatFunctionClass{}
_ functionClass = &fromBase64FunctionClass{}
_ functionClass = &toBase64FunctionClass{}
_ functionClass = &insertFuncFunctionClass{}
_ functionClass = &insertFunctionClass{}
_ functionClass = &instrFunctionClass{}
_ functionClass = &loadFileFunctionClass{}
)
Expand Down Expand Up @@ -137,7 +137,8 @@ var (
_ builtinFunc = &builtinFormatSig{}
_ builtinFunc = &builtinFromBase64Sig{}
_ builtinFunc = &builtinToBase64Sig{}
_ builtinFunc = &builtinInsertFuncSig{}
_ builtinFunc = &builtinInsertBinarySig{}
_ builtinFunc = &builtinInsertSig{}
_ builtinFunc = &builtinInstrSig{}
_ builtinFunc = &builtinInstrBinarySig{}
)
Expand Down Expand Up @@ -2611,70 +2612,105 @@ func splitToSubN(s string, n int) []string {
return subs
}

type insertFuncFunctionClass struct {
type insertFunctionClass struct {
baseFunctionClass
}

func (c *insertFuncFunctionClass) getFunction(ctx context.Context, args []Expression) (builtinFunc, error) {
func (c *insertFunctionClass) getFunction(ctx context.Context, args []Expression) (builtinFunc, error) {
if err := c.verifyArgs(args); err != nil {
return nil, errors.Trace(err)
}
sig := &builtinInsertFuncSig{newBaseBuiltinFunc(args, ctx)}
bf := newBaseBuiltinFuncWithTp(args, ctx, tpString, tpString, tpInt, tpInt, tpString)
bf.tp.Flen = mysql.MaxBlobWidth
SetBinFlagOrBinStr(args[0].GetType(), bf.tp)
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is a bug fix: for INSERT(str,pos,len,newstr), if str is a binary string, we should treat it like a byte array, otherwise we should treat it like a code point array. see builtinInsertBinarySig and builtinInsertSig for more detail

Copy link
Contributor

Choose a reason for hiding this comment

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

If we use args[0].GetType() after newBaseBuiltinFunc,
args[0] has already been wrapped with cast.

Copy link
Member Author

Choose a reason for hiding this comment

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

when arg[0] is a binary string, it will not be wrapped with a cast.

btw, I think wrapped cast functions should guarantee the correct type inference, and every function should do its type inference based on its children, no matter what expression the children is

SetBinFlagOrBinStr(args[3].GetType(), bf.tp)
var sig builtinFunc
Copy link
Contributor

Choose a reason for hiding this comment

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

move the definition of sig and err to line 2619.

if types.IsBinaryStr(args[0].GetType()) {
sig = &builtinInsertBinarySig{baseStringBuiltinFunc{bf}}
} else {
sig = &builtinInsertSig{baseStringBuiltinFunc{bf}}
}
return sig.setSelf(sig), nil
}

type builtinInsertFuncSig struct {
baseBuiltinFunc
type builtinInsertBinarySig struct {
baseStringBuiltinFunc
}

// eval evals a builtinInsertFuncSig.
// evalString evals INSERT(str,pos,len,newstr).
// See https://dev.mysql.com/doc/refman/5.6/en/string-functions.html#function_insert
func (b *builtinInsertFuncSig) eval(row []types.Datum) (d types.Datum, err error) {
args, err := b.evalArgs(row)
if err != nil {
return d, errors.Trace(err)
func (b *builtinInsertBinarySig) evalString(row []types.Datum) (string, bool, error) {
sc := b.ctx.GetSessionVars().StmtCtx

str, isNull, err := b.args[0].EvalString(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}

// Returns NULL if any argument is NULL.
if args[0].IsNull() || args[1].IsNull() || args[2].IsNull() || args[3].IsNull() {
return
pos, isNull, err := b.args[1].EvalInt(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}

str0, err := args[0].ToString()
if err != nil {
return d, errors.Trace(err)
length, isNull, err := b.args[2].EvalInt(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}
str := []rune(str0)
strLen := len(str)

posInt64, err := args[1].ToInt64(b.ctx.GetSessionVars().StmtCtx)
if err != nil {
return d, errors.Trace(err)
newstr, isNull, err := b.args[3].EvalString(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}
pos := int(posInt64)

lenInt64, err := args[2].ToInt64(b.ctx.GetSessionVars().StmtCtx)
if err != nil {
return d, errors.Trace(err)
strLength := int64(len(str))
if pos < 1 || pos > strLength {
Copy link
Contributor

Choose a reason for hiding this comment

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

this check can be moved to line 2654?

return str, false, nil
}
if length > strLength-pos+1 || length < 0 {
return str[0:pos-1] + newstr, false, nil
}
length := int(lenInt64)
return str[0:pos-1] + newstr + str[pos+length-1:], false, nil
}

newstr, err := args[3].ToString()
if err != nil {
return d, errors.Trace(err)
type builtinInsertSig struct {
baseStringBuiltinFunc
}

// evalString evals INSERT(str,pos,len,newstr).
// See https://dev.mysql.com/doc/refman/5.6/en/string-functions.html#function_insert
func (b *builtinInsertSig) evalString(row []types.Datum) (string, bool, error) {
sc := b.ctx.GetSessionVars().StmtCtx

str, isNull, err := b.args[0].EvalString(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}

var s string
if pos < 1 || pos > strLen {
s = str0
} else if length > strLen-pos+1 || length < 0 {
s = string(str[0:pos-1]) + newstr
} else {
s = string(str[0:pos-1]) + newstr + string(str[pos+length-1:])
pos, isNull, err := b.args[1].EvalInt(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}

d.SetString(s)
return d, nil
length, isNull, err := b.args[2].EvalInt(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}

newstr, isNull, err := b.args[3].EvalString(row, sc)
if isNull || err != nil {
return "", true, errors.Trace(err)
}

runes := []rune(str)
runeLength := int64(len(runes))

if pos < 1 || pos > runeLength {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

return str, false, nil
}
if length > runeLength-pos+1 || length < 0 {
return string(runes[0:pos-1]) + newstr, false, nil
}
return string(runes[0:pos-1]) + newstr + string(runes[pos+length-1:]), false, nil
Copy link
Member

@jackysp jackysp Sep 6, 2017 •
edited

Choose a reason for hiding this comment

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

Use fmt.Sprintf instead?

Copy link
Member Author

@zz-jason zz-jason Sep 6, 2017 •
edited

Choose a reason for hiding this comment

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

package main

import "fmt"
import "time"

func main() {
        str := ""
        str1 := "aaaa"
        str2 := "bbbb"
        str3 := "cccc"

        start := time.Now()
        for i, round := 0, 1000000; i < round; i++ {
                str = str1 + str2 + str3
        }
        fmt.Printf("%vns\n", time.Now().Sub(start).Nanoseconds())
        fmt.Printf("%s\n", str)

        start = time.Now()
        for i, round := 0, 1000000; i < round; i++ {
                str = fmt.Sprintf("%s%s%s", str1, str2, str3)
        }
        fmt.Printf("%vns\n", time.Now().Sub(start).Nanoseconds())
        fmt.Printf("%s\n", str)
}
$go build a.go
$./a
56394494ns
aaaabbbbcccc
279845006ns
aaaabbbbcccc

It seems fmt.Sprintf() has less efficiency.

}

type instrFunctionClass struct {
Expand Down
2 changes: 2 additions & 0 deletions expression/builtin_string_test.go
View file
Open in desktop
Expand Up @@ -1577,6 +1577,8 @@ func (s *testEvaluatorSuite) TestInsert(c *C) {
for _, test := range tests {
f, err := fc.getFunction(s.ctx, datumsToConstants(types.MakeDatums(test.args...)))
c.Assert(err, IsNil)
c.Assert(f, NotNil)
c.Assert(f.canBeFolded(), IsTrue)
result, err := f.eval(nil)
c.Assert(err, IsNil)
if test.expect == nil {
Expand Down
6 changes: 6 additions & 0 deletions expression/integration_test.go
View file
Open in desktop
Expand Up @@ -834,6 +834,12 @@ func (s *testIntegrationSuite) TestStringBuiltin(c *C) {
result.Check(testkit.Rows("'aaaa' '' '\"\"' '\n\n'"))
result = tk.MustQuery(`select quote(0121), quote(0000), quote("中文"), quote(NULL);`)
result.Check(testkit.Rows("'121' '0' '中文' <nil>"))

// for insert
result = tk.MustQuery(`select insert("中文", 1, 1, cast("aaa" as binary)), insert("ba", -1, 1, "aaa"), insert("ba", 1, 100, "aaa"), insert("ba", 100, 1, "aaa");`)
result.Check(testkit.Rows("aaa文 ba aaa ba"))
result = tk.MustQuery(`select insert("bb", NULL, 1, "aa"), insert("bb", 1, NULL, "aa"), insert(NULL, 1, 1, "aaa"), insert("bb", 1, 1, NULL);`)
result.Check(testkit.Rows("<nil> <nil> <nil> <nil>"))
}

func (s *testIntegrationSuite) TestEncryptionBuiltin(c *C) {
Expand Down
5 changes: 5 additions & 0 deletions plan/typeinfer_test.go
View file
Open in desktop
Expand Up @@ -424,6 +424,11 @@ func (s *testPlanSuite) createTestCase4StrFuncs() []typeInferTestCase {
{"quote(c_bigint_d )", mysql.TypeVarString, charset.CharsetUTF8, 0, 42, types.UnspecifiedLength},
{"quote(c_float_d )", mysql.TypeVarString, charset.CharsetUTF8, 0, 26, types.UnspecifiedLength},
{"quote(c_double_d )", mysql.TypeVarString, charset.CharsetUTF8, 0, 46, types.UnspecifiedLength},

{"insert(c_varchar, c_int_d, c_int_d, c_varchar)", mysql.TypeLongBlob, charset.CharsetUTF8, 0, mysql.MaxBlobWidth, types.UnspecifiedLength},
{"insert(c_varchar, c_int_d, c_int_d, c_binary)", mysql.TypeLongBlob, charset.CharsetBin, mysql.BinaryFlag, mysql.MaxBlobWidth, types.UnspecifiedLength},
{"insert(c_binary, c_int_d, c_int_d, c_varchar)", mysql.TypeLongBlob, charset.CharsetBin, mysql.BinaryFlag, mysql.MaxBlobWidth, types.UnspecifiedLength},
{"insert(c_binary, c_int_d, c_int_d, c_binary)", mysql.TypeLongBlob, charset.CharsetBin, mysql.BinaryFlag, mysql.MaxBlobWidth, types.UnspecifiedLength},
}
}

Expand Down