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 ELT and FIELD #15249

Merged
merged 9 commits into from
Apr 1, 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.

17 changes: 17 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,23 @@ func (asm *assembler) Fn_BIT_LENGTH() {
}, "FN BIT_LENGTH VARCHAR(SP-1)")
}

func (asm *assembler) Fn_ELT(args int, tt sqltypes.Type, tc collations.TypedCollation) {
asm.adjustStack(-args + 1)
asm.emit(func(env *ExpressionEnv) int {
i := env.vm.stack[env.vm.sp-args].(*evalInt64)

if i.i < 1 || int(i.i) >= args || env.vm.stack[env.vm.sp-args+int(i.i)] == nil {
env.vm.stack[env.vm.sp-args] = nil
} else {
b := env.vm.stack[env.vm.sp-args+int(i.i)].(*evalBytes)
env.vm.stack[env.vm.sp-args] = env.vm.arena.newEvalRaw(b.bytes, tt, tc)
}

env.vm.sp -= args - 1
return 1
}, "FN ELT INT64(SP-%d) VARCHAR(SP-%d)...VARCHAR(SP-1)", args, args-1)
}

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

Expand Down
114 changes: 114 additions & 0 deletions go/vt/vtgate/evalengine/fn_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@
)

type (
builtinElt struct {
CallExpr
collate collations.ID
}

builtinInsert struct {
CallExpr
collate collations.ID
Expand Down Expand Up @@ -112,6 +117,7 @@
}
)

var _ IR = (*builtinElt)(nil)
var _ IR = (*builtinInsert)(nil)
var _ IR = (*builtinChangeCase)(nil)
var _ IR = (*builtinCharLength)(nil)
Expand All @@ -127,6 +133,114 @@
var _ IR = (*builtinPad)(nil)
var _ IR = (*builtinTrim)(nil)

func (call *builtinElt) eval(env *ExpressionEnv) (eval, error) {
var ca collationAggregation
tt := sqltypes.VarChar

args, err := call.args(env)
if err != nil {
return nil, err
}

if args[0] == nil {
return nil, nil
}

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L147 was not covered by tests

i := evalToInt64(args[0]).i
if i < 1 || i >= int64(len(args)) || args[i] == nil {
return nil, nil
}

for _, arg := range args[1:] {
if arg == nil {
continue
}

tt = concatSQLType(arg.SQLType(), tt)
err = ca.add(evalCollation(arg), env.collationEnv)
if err != nil {
return nil, err
}
}

tc := ca.result()
// If we only had numbers, we instead fall back to the default

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L167 was not covered by tests
// collation instead of using the numeric collation.
if tc.Coercibility == collations.CoerceNumeric {
tc = typedCoercionCollation(tt, call.collate)
}

b, err := evalToVarchar(args[i], tc.Collation, true)
if err != nil {
return nil, err
}

return newEvalRaw(tt, b.bytes, b.col), nil
}

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L180 was not covered by tests
func (call *builtinElt) compile(c *compiler) (ctype, error) {
args := make([]ctype, len(call.Arguments))

var ca collationAggregation
tt := sqltypes.VarChar

var skip *jump
for i, arg := range call.Arguments {
var err error
args[i], err = arg.compile(c)
if err != nil {
return ctype{}, nil
}

if i == 0 {
skip = c.compileNullCheck1(args[i])
continue

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L197 was not covered by tests
}

tt = concatSQLType(args[i].Type, tt)
err = ca.add(args[i].Col, c.env.CollationEnv())
if err != nil {
return ctype{}, err
}
}

tc := ca.result()
// If we only had numbers, we instead fall back to the default

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L208 was not covered by tests
// collation instead of using the numeric collation.
if tc.Coercibility == collations.CoerceNumeric {
tc = typedCoercionCollation(tt, call.collate)
}

_ = c.compileToInt64(args[0], len(args))

for i, arg := range args[1:] {
offset := len(args) - (i + 1)
skip := c.compileNullCheckOffset(arg, offset)

switch arg.Type {
case sqltypes.VarBinary, sqltypes.Binary, sqltypes.Blob:
if tc.Collation != collations.CollationBinaryID {
c.asm.Convert_xce(offset, arg.Type, tc.Collation)
}
case sqltypes.VarChar, sqltypes.Char, sqltypes.Text:
fromCharset := colldata.Lookup(arg.Col.Collation).Charset()
toCharset := colldata.Lookup(tc.Collation).Charset()
if fromCharset != toCharset && !toCharset.IsSuperset(fromCharset) {

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L228 was not covered by tests
c.asm.Convert_xce(offset, arg.Type, tc.Collation)
}
default:
c.asm.Convert_xce(offset, arg.Type, tc.Collation)
}
Comment on lines +408 to +421
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we create a common function for this? We are now using the same logic in 3 functions.


c.asm.jumpDestination(skip)
}

c.asm.Fn_ELT(len(args), tt, tc)
c.asm.jumpDestination(skip)

return ctype{Type: tt, Col: tc, Flag: flagNullable}, nil
}

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

Expand Down
51 changes: 51 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: FnElt},
{Run: FnInsert},
{Run: FnLower},
{Run: FnUpper},
Expand Down Expand Up @@ -1315,6 +1316,56 @@ var JSONExtract_Schema = []*querypb.Field{
},
}

func FnElt(yield Query) {
for _, s1 := range inputStrings {
for _, n := range inputBitwise {
yield(fmt.Sprintf("ELT(%s, %s)", n, s1), nil)
}
}

for _, s1 := range inputStrings {
for _, s2 := range inputStrings {
for _, n := range inputBitwise {
yield(fmt.Sprintf("ELT(%s, %s, %s)", n, s1, s2), nil)
}
}
}

for _, s1 := range inputStrings {
for _, s2 := range inputStrings {
for _, s3 := range inputStrings {
for _, n := range inputBitwise {
yield(fmt.Sprintf("ELT(%s, %s, %s, %s)", n, s1, s2, s3), nil)
}
}
}
}

validIndex := []string{
"1",
"2",
"3",
}
for _, s1 := range inputStrings {
for _, s2 := range inputStrings {
for _, s3 := range inputStrings {
for _, n := range validIndex {
yield(fmt.Sprintf("ELT(%s, %s, %s, %s)", n, s1, s2, s3), nil)
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing to look at with stuff like this, is how much test runtime we're adding. It might be too much with all these permutations? How long does it take to run these tests?

Copy link
Member Author

@beingnoble03 beingnoble03 Feb 15, 2024

Choose a reason for hiding this comment

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

about 24 sec, in the comparison test. total 567338 number of tests.
CONCAT_WS takes 12 sec, total 229438 number of tests

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 we should reduce this then, that's a lot of time for a single subtest here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the test for 4 inputs. Total test time reduced to 6 sec, total 56234 number of tests.


mysqlDocSamples := []string{
"ELT(1, 'Aa', 'Bb', 'Cc', 'Dd')",
"ELT(4, 'Aa', 'Bb', 'Cc', 'Dd')",
}

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

func FnInsert(yield Query) {
for _, s := range insertStrings {
for _, ns := range insertStrings {
Expand Down
5 changes: 5 additions & 0 deletions go/vt/vtgate/evalengine/translate_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@
return nil, argError(method)
}
return &builtinPad{CallExpr: call, collate: ast.cfg.Collation, left: method == "lpad"}, nil
case "elt":
if len(args) < 2 {

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

View check run for this annotation

Codecov / codecov/patch

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

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