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: Support built-in MySQL function for CONV function #11566

Closed
wants to merge 5 commits into from
Closed

evalengine: Support built-in MySQL function for CONV function #11566

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
88b3645
add support for CONV
Weijun-H Oct 24, 2022
f1654a3
failed to test hex literal
Weijun-H Nov 1, 2022
efbaff6
support hex literal
Weijun-H Nov 2, 2022
67575f3
support more tests
Weijun-H Nov 3, 2022
654abcb
support negetive base
Weijun-H Nov 9, 2022
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
1 change: 1 addition & 0 deletions go/vt/vtgate/evalengine/func.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ var builtinFunctions = map[string]builtin{
"bit_length": builtinBitLength{},
"ascii": builtinASCII{},
"repeat": builtinRepeat{},
"conv": builtinConv{},
}

var builtinFunctionsRewrite = map[string]builtinRewrite{
Expand Down
48 changes: 48 additions & 0 deletions go/vt/vtgate/evalengine/integration/string_fun_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,3 +204,51 @@ func TestBuiltinRepeat(t *testing.T) {

}
}

func TestBuiltinConv(t *testing.T) {
var conn = mysqlconn(t)
defer conn.Close()
cases := []string{
"-5.1",
"-5.9",
"0xa21 + '1'",
"-0xa21 + '1'",
"10",
"10 + '10' + 10",
"10 + '10' - 10",
"-10",
"'10'",
"10+'10'+'10a'+X'0a'",
"10 / 10",
"X'0FFFFFFFFFFFFFF'",
"99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999",
"-99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999",
}
Copy link
Member

Choose a reason for hiding this comment

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

These still aren't strong enough cases. What happens if we have a + in the beginning? What if we have 2 of those in the beginning? What if it is a string with a + and not a number?

bases := []string{
"-1",
"1",
"2",
"4",
"8",
"10",
"16",
"32",
"64",
}

for _, num := range cases {
for _, fromBase := range bases {
for i := range bases {
toBase := bases[i]
query := fmt.Sprintf("CONV(%s, %s, %s)", num, fromBase, toBase)
compareRemoteExpr(t, conn, query)

toBase = bases[len(bases)-1-i]
query = fmt.Sprintf("CONV(%s, %s, %s)", num, fromBase, toBase)
compareRemoteExpr(t, conn, query)
}
}

}

}
111 changes: 111 additions & 0 deletions go/vt/vtgate/evalengine/string.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ package evalengine

import (
"bytes"
"fmt"
"math"
"regexp"
"strconv"
"strings"

"vitess.io/vitess/go/mysql/collations"
"vitess.io/vitess/go/sqltypes"
Expand Down Expand Up @@ -265,3 +270,109 @@ func (builtinRepeat) typeof(env *ExpressionEnv, args []Expr) (sqltypes.Type, fla

return sqltypes.VarChar, f1
}

type builtinConv struct {
}

func (builtinConv) call(env *ExpressionEnv, args []EvalResult, result *EvalResult) {
const MaxUint = 18446744073709551615
var fromNum uint64
var fromNumNeg uint64
var isNeg bool
var rawString string
inarg := &args[0]
inarg2 := &args[1]
inarg3 := &args[2]
fromBase := inarg2.int64()
toBase := inarg3.int64()
fromNum = 0
t := inarg.typeof()

if inarg.isNull() || inarg2.isNull() || inarg3.isNull() || fromBase < 2 || fromBase > 36 || toBase < 2 || toBase > 36 {
result.setNull()
return
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the checks inarg2.isNull() or inarge3.isNull() is required. The values of fromBase and toBase would be 0 if they were null respectively, so the condition would still be true.


rawString = string(inarg.toRawBytes())
rawString = strings.ToLower(rawString)

if t == sqltypes.Float64 {
for i, c := range rawString {
if c == '-' {
continue
}
if (fromBase <= 9 && c >= '0' && c <= rune('0'+fromBase)) || (fromBase > 9 && ((c >= '0' && c <= '9') || (c >= 'a' && c <= rune('a'+fromBase-9)))) {
continue
} else {
rawString = rawString[:i]
break
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is there any specific reason why you need the float type as a separate case?


re, _ := regexp.Compile(`[+-]?[0-9.x]+[a-vA-Vx]*`)
for _, num := range re.FindAllString(rawString, -1) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need all substring matches? Don't you only need the first one? In what case would have more than 1 matches?

isNeg = false
reFindDot, _ := regexp.Compile(`\.`)
reFindNeg, _ := regexp.Compile(`^-[0-9.]+[a-vA-V]*`)
if reFindDot.MatchString(num) {
temp, _ := strconv.ParseFloat(num, 64)
temp = math.Trunc(temp)
num = fmt.Sprint(int64(temp))
}
if reFindNeg.MatchString(num) {
isNeg = true
num = num[1:]
}

if transNum, err := strconv.ParseUint(num, int(fromBase), 64); err == nil {
if isNeg {
fromNumNeg = fromNumNeg + transNum
} else {
fromNum = fromNum + transNum
}
} else if strings.Contains(err.Error(), "value out of range") {
if isNeg {
fromNumNeg = MaxUint

} else {
fromNum = MaxUint
}
} else {
fromNum = 0
break
}
}

if fromNumNeg < fromNum {
fromNum = fromNum - fromNumNeg
} else if fromNumNeg == MaxUint {
fromNum = 0
} else {
fromNum = fromNumNeg - fromNum
}
var toNum string
if isNeg {
temp := strconv.FormatUint(uint64(-fromNum), int(toBase))
toNum = strings.ToUpper(temp)
} else {
temp := strconv.FormatUint(fromNum, int(toBase))
toNum = strings.ToUpper(temp)
}

inarg.makeTextualAndConvert(env.DefaultCollation)
Copy link
Member

Choose a reason for hiding this comment

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

From what I can see, there really is no need to convert inarg to the environment's collation. You can just extract the collation from inarg and change the collation and then type cast the result directly using that.
Assuming that the goal is to have the collation finally be the default collation with the coercability and repertoire being inherited from the first argument.
In this case, you can even remove the inarg.makeUnsignedIntegral too, because you only need the raw bytes, so that is also only being done for the collation changes.

However, I don't know what coercability or repotoire we want. Is there some way to verify from MySQL what it does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can just extract the collation from inarg and change the collation and then type cast the result directly using that.

I have no idea how to do in this way.

result.setString(toNum, inarg.collation())
}

func (builtinConv) typeof(env *ExpressionEnv, args []Expr) (sqltypes.Type, flag) {
if len(args) != 3 {
throwArgError("CONV")
}
_, f1 := args[0].typeof(env)
_, f2 := args[1].typeof(env)
// typecheck the right-hand argument but ignore its flags
args[1].typeof(env)
args[2].typeof(env)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing typecheck for args[2].

return sqltypes.VarChar, f1 & f2
}