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

evalEngine: Implement INSTR #15127

Closed
wants to merge 1 commit into from
Closed

evalEngine: Implement INSTR #15127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions go/vt/vtgate/evalengine/cached_size.go
View file
Open in desktop

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

12 changes: 12 additions & 0 deletions go/vt/vtgate/evalengine/compiler_asm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,18 @@ func (asm *assembler) Mod_dd() {
}, "MOD DECIMAL(SP-2), DECIMAL(SP-1)")
}

func (asm *assembler) Fn_INSTR() {
asm.adjustStack(-1)

asm.emit(func(env *ExpressionEnv) int {
str := env.vm.stack[env.vm.sp-2].(*evalBytes)
substr := env.vm.stack[env.vm.sp-1].(*evalBytes)

env.vm.stack[env.vm.sp-1] = env.vm.arena.newEvalInt64(instrIndex(str, substr))
return 1
}, "FN INSTR VARCHAR(SP-2) VARCHAR(SP-1)")
}

func (asm *assembler) Fn_ASCII() {
asm.emit(func(env *ExpressionEnv) int {
arg := env.vm.stack[env.vm.sp-1].(*evalBytes)
Expand Down
77 changes: 77 additions & 0 deletions go/vt/vtgate/evalengine/fn_string.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
CallExpr
}

builtinInstr struct {
CallExpr
collate collations.ID
}

builtinASCII struct {
CallExpr
}
Expand Down Expand Up @@ -99,6 +104,7 @@
var _ IR = (*builtinChangeCase)(nil)
var _ IR = (*builtinCharLength)(nil)
var _ IR = (*builtinLength)(nil)
var _ IR = (*builtinInstr)(nil)
var _ IR = (*builtinASCII)(nil)
var _ IR = (*builtinOrd)(nil)
var _ IR = (*builtinBitLength)(nil)
Expand Down Expand Up @@ -199,6 +205,77 @@
return c.compileFn_length(call.Arguments[0], c.asm.Fn_LENGTH)
}

func instrIndex(str *evalBytes, substr *evalBytes) int64 {
// Case sensitive if one of the strings is binary string
if !str.isBinary() && !substr.isBinary() {
str.bytes = bytes.ToLower(str.bytes)
substr.bytes = bytes.ToLower(substr.bytes)
}

pos := bytes.Index(str.bytes, substr.bytes) + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a reason why INSTR is not implemented yet, and that is that we need to add functionality to the go/mysql/collations package to do the searching for a substring. This implementation here is wrong and doesn't consider collations correctly. It assumes that it's all only ASCII by the ToLower and doesn't handle non ASCII characters this way, nor does it handle any of the non-Unicode encodings here.

return int64(pos)
}

func (call *builtinInstr) eval(env *ExpressionEnv) (eval, error) {
arg1, arg2, err := call.arg2(env)
if err != nil {
return nil, err

Check warning on line 222 in go/vt/vtgate/evalengine/fn_string.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/evalengine/fn_string.go#L222 

Added line #L222 was not covered by tests
}
if arg1 == nil || arg2 == nil {
return nil, nil
}

str, ok := arg1.(*evalBytes)
if !ok {
str, err = evalToVarchar(arg1, call.collate, true)
if err != nil {
return nil, err

Check warning on line 232 in go/vt/vtgate/evalengine/fn_string.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/evalengine/fn_string.go#L232 

Added line #L232 was not covered by tests
}
}

substr, ok := arg2.(*evalBytes)
if !ok {
substr, err = evalToVarchar(arg2, call.collate, true)
if err != nil {
return nil, err

Check warning on line 240 in go/vt/vtgate/evalengine/fn_string.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/evalengine/fn_string.go#L240 

Added line #L240 was not covered by tests
}
}

return newEvalInt64(instrIndex(str, substr)), nil
}

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

Check warning on line 250 in go/vt/vtgate/evalengine/fn_string.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/evalengine/fn_string.go#L250 

Added line #L250 was not covered by tests
}

skip1 := c.compileNullCheck1(arg1)

switch {
case arg1.isTextual():
default:
c.asm.Convert_xce(1, sqltypes.VarChar, call.collate)
}

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

Check warning on line 263 in go/vt/vtgate/evalengine/fn_string.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/evalengine/fn_string.go#L263 

Added line #L263 was not covered by tests
}

skip2 := c.compileNullCheck1(arg2)

switch {
case arg2.isTextual():
default:
c.asm.Convert_xce(1, sqltypes.VarChar, call.collate)
}

c.asm.Fn_INSTR()
c.asm.jumpDestination(skip1, skip2)
return ctype{Type: sqltypes.Int64, Col: collationNumeric}, nil
}

func (call *builtinBitLength) eval(env *ExpressionEnv) (eval, error) {
arg, err := call.arg1(env)
if err != nil {
Expand Down
36 changes: 36 additions & 0 deletions go/vt/vtgate/evalengine/testcases/cases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ var Cases = []TestCase{
{Run: FnUpper},
{Run: FnCharLength},
{Run: FnLength},
{Run: FnInstr},
{Run: FnBitLength},
{Run: FnAscii},
{Run: FnOrd},
Expand Down Expand Up @@ -1339,6 +1340,41 @@ func FnLength(yield Query) {
}
}

func FnInstr(yield Query) {
for _, str := range inputStrings {
for _, substr := range inputStrings {
yield(fmt.Sprintf("INSTR(%s, %s)", str, substr), nil)
}
}

cases := []struct {
str string
substr string
}{
{"'ACABAB'", "'AB'"},
{"'ABABAB'", "'AB'"},
{"'ABABAB'", "'ab'"},
{"'ABABAB'", "'ba'"},
{"'CBDASD'", "'ab'"},
{"'ABABAB'", "''"},
{"'ABABAB'", ""},
{"1233", "23"},
{"0x616162", "0x6162"},
{"0x616162", "0x4141"},
{"'AAB'", "123"},
{"123", "'ABC'"},
{"_binary'FOOBAR'", "'AR'"},
{"_binary'FOOBAR'", "BINARY 'AR'"},
{"BINARY 'FOOBAR'", "'ar'"},
{"'foobarbar'", "'bar'"},
{"'xbar'", "'foobar'"},
}

for _, tc := range cases {
yield(fmt.Sprintf("INSTR(%s, %s)", tc.str, tc.substr), nil)
}
}

func FnBitLength(yield Query) {
for _, str := range inputStrings {
yield(fmt.Sprintf("BIT_LENGTH(%s)", str), nil)
Expand Down
5 changes: 5 additions & 0 deletions go/vt/vtgate/evalengine/translate_builtin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,11 @@
return nil, argError(method)
}
return &builtinLength{CallExpr: call}, nil
case "instr":
if len(args) != 2 {
return nil, argError(method)

Check warning on line 290 in go/vt/vtgate/evalengine/translate_builtin.go

View check run for this annotation

Codecov / codecov/patch

go/vt/vtgate/evalengine/translate_builtin.go#L290 

Added line #L290 was not covered by tests
}
return &builtinInstr{CallExpr: call, collate: ast.cfg.Collation}, nil
case "bit_length":
if len(args) != 1 {
return nil, argError(method)
Expand Down
Loading