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

evalEngine: Implement string INSERT #15201

Merged
merged 8 commits into from
Feb 12, 2024
Merged
12 changes: 12 additions & 0 deletions go/vt/vtgate/evalengine/cached_size.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions go/vt/vtgate/evalengine/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,15 @@ func (c *compiler) compileNullCheck3(arg1, arg2, arg3 ctype) *jump {
return nil
}

func (c *compiler) compileNullCheck4(arg1, arg2, arg3, arg4 ctype) *jump {
if arg1.nullable() || arg2.nullable() || arg3.nullable() || arg4.nullable() {
j := c.asm.jumpFrom()
c.asm.NullCheck4(j)
return j
}
return nil
}

func (c *compiler) compileNullCheckArg(ct ctype, offset int) *jump {
if ct.nullable() {
j := c.asm.jumpFrom()
Expand Down
26 changes: 26 additions & 0 deletions go/vt/vtgate/evalengine/compiler_asm.go
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,21 @@ func (asm *assembler) Fn_BIT_LENGTH() {
}, "FN BIT_LENGTH VARCHAR(SP-1)")
}

func (asm *assembler) Fn_INSERT(col collations.TypedCollation) {
asm.adjustStack(-3)

asm.emit(func(env *ExpressionEnv) int {
str := env.vm.stack[env.vm.sp-4].(*evalBytes)
pos := env.vm.stack[env.vm.sp-3].(*evalInt64).i
l := env.vm.stack[env.vm.sp-2].(*evalInt64).i
newstr := env.vm.stack[env.vm.sp-1].(*evalBytes)

env.vm.stack[env.vm.sp-4] = env.vm.arena.newEvalText(insert(str, newstr, int(pos), int(l)), col)
env.vm.sp -= 3
return 1
}, "FN INSERT VARCHAR(SP-4) INT64(SP-3) INT64(SP-2) VARCHAR(SP-1)")
}

func (asm *assembler) Fn_LUCASE(upcase bool) {
if upcase {
asm.emit(func(env *ExpressionEnv) int {
Expand Down Expand Up @@ -3147,6 +3162,17 @@ func (asm *assembler) NullCheck3(j *jump) {
}, "NULLCHECK SP-1, SP-2, SP-3")
}

func (asm *assembler) NullCheck4(j *jump) {
asm.emit(func(env *ExpressionEnv) int {
if env.vm.stack[env.vm.sp-4] == nil || env.vm.stack[env.vm.sp-3] == nil || env.vm.stack[env.vm.sp-2] == nil || env.vm.stack[env.vm.sp-1] == nil {
env.vm.stack[env.vm.sp-4] = nil
env.vm.sp -= 3
return j.offset()
}
return 1
}, "NULLCHECK SP-1, SP-2, SP-3, SP-4")
}

func (asm *assembler) NullCheckArg(j *jump, offset int) {
asm.emit(func(env *ExpressionEnv) int {
if env.vm.stack[env.vm.sp-1] == nil {
Expand Down
110 changes: 110 additions & 0 deletions go/vt/vtgate/evalengine/fn_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package evalengine

import (
"bytes"
"math"

"vitess.io/vitess/go/mysql/collations"
"vitess.io/vitess/go/mysql/collations/charset"
Expand All @@ -29,6 +30,11 @@ import (
)

type (
builtinInsert struct {
CallExpr
collate collations.ID
}

builtinChangeCase struct {
CallExpr
upcase bool
Expand Down Expand Up @@ -106,6 +112,7 @@ type (
}
)

var _ IR = (*builtinInsert)(nil)
var _ IR = (*builtinChangeCase)(nil)
var _ IR = (*builtinCharLength)(nil)
var _ IR = (*builtinLength)(nil)
Expand All @@ -120,6 +127,109 @@ var _ IR = (*builtinLeftRight)(nil)
var _ IR = (*builtinPad)(nil)
var _ IR = (*builtinTrim)(nil)

func insert(str, newstr *evalBytes, pos, l int) []byte {
pos--

cs := colldata.Lookup(str.col.Collation).Charset()
strLen := charset.Length(cs, str.bytes)

if pos < 0 || strLen <= pos {
return str.bytes
}
if l < 0 {
l = strLen
}

front := charset.Slice(cs, str.bytes, 0, pos)
var back []byte
if pos <= math.MaxInt-l && pos+l < strLen {
Copy link
Member Author

Choose a reason for hiding this comment

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

pos <= math.MaxInt-l can be used here to check overflow, as both pos and l are positive (checks added above).

back = charset.Slice(cs, str.bytes, pos+l, strLen)
}

res := make([]byte, len(front)+len(newstr.bytes)+len(back))

copy(res[:len(front)], front)
copy(res[len(front):], newstr.bytes)
copy(res[len(front)+len(newstr.bytes):], back)
Copy link
Contributor

Choose a reason for hiding this comment

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

@vmg Do you think it's worth extending charset.Slice to allow passing in an existing output []byte or is that a bit premature just for this function?

Copy link
Collaborator

@vmg vmg Feb 12, 2024

Choose a reason for hiding this comment

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

I think it would make for a more awkward API, don't you think? Right now Slice doesn't allocate (it returns a sub-slice of the input), so if we add an out parameter, we'd have to make it always allocate or have two code paths in the function for when out is nil.

Furthermore, if the Slice API had an out parameter, it would allocate more than once when used for INSERT, since we wouldn't know the required capacity of the out buffer, so it'd have to grow it at least twice.

With the current non-allocating Slice, we just call the function twice and then we get to compose the final result outside of the function with a single allocation, since we know the final size. I quite dig the version of the code as it is written.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yeah, good point. It's already non-allocating so it doesn't really help in any way.


return res
}

func (call *builtinInsert) eval(env *ExpressionEnv) (eval, error) {
args, err := call.args(env)
if err != nil {
return nil, err
}
if args[0] == nil || args[1] == nil || args[2] == nil || args[3] == nil {
return nil, nil
}
beingnoble03 marked this conversation as resolved.
Show resolved Hide resolved

str, ok := args[0].(*evalBytes)
if !ok {
str, err = evalToVarchar(args[0], call.collate, true)
if err != nil {
return nil, err
}
}

pos := evalToInt64(args[1]).i
l := evalToInt64(args[2]).i

newstr, err := evalToVarchar(args[3], str.col.Collation, true)
if err != nil {
return nil, err
}

return newEvalText(insert(str, newstr, int(pos), int(l)), str.col), nil
}

func (call *builtinInsert) compile(c *compiler) (ctype, error) {
str, err := call.Arguments[0].compile(c)
if err != nil {
return ctype{}, err
}

pos, err := call.Arguments[1].compile(c)
if err != nil {
return ctype{}, err
}

l, err := call.Arguments[2].compile(c)
if err != nil {
return ctype{}, err
}

newstr, err := call.Arguments[3].compile(c)
if err != nil {
return ctype{}, err
}

skip := c.compileNullCheck4(str, pos, l, newstr)

_ = c.compileToInt64(pos, 3)
_ = c.compileToInt64(l, 2)

if err != nil {
return ctype{}, nil
}

col := str.Col

switch {
case str.isTextual():
default:
c.asm.Convert_xce(4, sqltypes.VarChar, c.collation)
col = typedCoercionCollation(sqltypes.VarChar, c.collation)
}

c.asm.Convert_xce(1, sqltypes.VarChar, col.Collation)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this conversion should be conditional? This is only needed if it's not already a string?

Copy link
Member Author

Choose a reason for hiding this comment

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

done.


c.asm.Fn_INSERT(col)
c.asm.jumpDestination(skip)

return ctype{Type: sqltypes.VarChar, Col: col, Flag: flagNullable}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there cases where this function can return NULL if all inputs are not NULL?

Copy link
Member Author

@beingnoble03 beingnoble03 Feb 11, 2024

Choose a reason for hiding this comment

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

Yes, if the resultant string size is greater than max_allowed_packet, I think I forgot to add the validation for that. Pushed now. 😅

}

func (call *builtinChangeCase) eval(env *ExpressionEnv) (eval, error) {
arg, err := call.arg1(env)
if err != nil {
Expand Down
23 changes: 23 additions & 0 deletions go/vt/vtgate/evalengine/testcases/cases.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ var Cases = []TestCase{
{Run: TupleComparisons},
{Run: Comparisons},
{Run: InStatement},
{Run: FnInsert},
{Run: FnLower},
{Run: FnUpper},
{Run: FnCharLength},
Expand Down Expand Up @@ -1314,6 +1315,28 @@ var JSONExtract_Schema = []*querypb.Field{
},
}

func FnInsert(yield Query) {
for _, s := range insertStrings {
for _, ns := range insertStrings {
for _, l := range inputBitwise {
for _, p := range inputBitwise {
yield(fmt.Sprintf("INSERT(%s, %s, %s, %s)", s, p, l, ns), nil)
}
}
}
}

mysqlDocSamples := []string{
"INSERT('Quadratic', 3, 4, 'What')",
"INSERT('Quadratic', -1, 4, 'What')",
"INSERT('Quadratic', 3, 100, 'What')",
}

for _, q := range mysqlDocSamples {
yield(q, nil)
}
}

func FnLower(yield Query) {
for _, str := range inputStrings {
yield(fmt.Sprintf("LOWER(%s)", str), nil)
Expand Down
34 changes: 34 additions & 0 deletions go/vt/vtgate/evalengine/testcases/inputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,40 @@ var inputStrings = []string{
// "_ucs2 'AabcÅå'",
}

var insertStrings = []string{
"NULL",
"\"\"",
"\"a\"",
"\"abc\"",
"1",
"-1",
"0123",
"0xAACC",
"3.1415926",
// MySQL has broken behavior for these inputs,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can reference mysql/mysql-server#517 here so we know what this is about in the future too.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

// "\"Å å\"",
// "\"中文测试\"",
// "\"日本語テスト\"",
// "\"한국어 시험\"",
// "\"😊😂🤢\"",
// "_utf8mb4 'abcABCÅå'",
"DATE '2022-10-11'",
"TIME '11:02:23'",
"'123'",
"9223372036854775807",
"-9223372036854775808",
"999999999999999999999999",
"-999999999999999999999999",
"_binary 'Müller' ",
"_latin1 0xFF",
// TODO: support other multibyte encodings
// "_dec8 'ÒòÅå'",
// "_utf8mb3 'abcABCÅå'",
// "_utf16 'AabcÅå'",
// "_utf32 'AabcÅå'",
// "_ucs2 'AabcÅå'",
}

var inputConversionTypes = []string{
"BINARY", "BINARY(1)", "BINARY(0)", "BINARY(16)", "BINARY(-1)",
"CHAR", "CHAR(1)", "CHAR(0)", "CHAR(16)", "CHAR(-1)",
Expand Down
29 changes: 29 additions & 0 deletions go/vt/vtgate/evalengine/translate_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,35 @@ func (ast *astCompiler) translateCallable(call sqlparser.Callable) (IR, error) {
return &builtinRegexpReplace{
CallExpr: CallExpr{Arguments: args, Method: "REGEXP_REPLACE"},
}, nil

case *sqlparser.InsertExpr:
str, err := ast.translateExpr(call.Str)
if err != nil {
return nil, err
}

pos, err := ast.translateExpr(call.Pos)
if err != nil {
return nil, err
}

len, err := ast.translateExpr(call.Len)
if err != nil {
return nil, err
}

newstr, err := ast.translateExpr(call.NewStr)
if err != nil {
return nil, err
}

args := []IR{str, pos, len, newstr}

var cexpr = CallExpr{Arguments: args, Method: "INSERT"}
return &builtinInsert{
CallExpr: cexpr,
collate: ast.cfg.Collation,
}, nil
default:
return nil, translateExprNotSupported(call)
}
Expand Down
Loading