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

Set udv allow more expressions #6964

Merged
merged 7 commits into from
Nov 2, 2020
Merged

Set udv allow more expressions #6964

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
126435b
allow all expr to be evaluated by tablet
harshit-gangal Oct 28, 2020
bcd519a
added udv with input
harshit-gangal Oct 28, 2020
3eeade1
allow datetime in evalengine
harshit-gangal Oct 28, 2020
200032a
added e2e test for udv with time function
harshit-gangal Oct 28, 2020
4cff195
evalengine slight refactor
harshit-gangal Oct 29, 2020
5ad63e8
store sqlvalue directly from result in session for column type expr
harshit-gangal Oct 29, 2020
589f9b2
allow all types in evalresult and conversion
harshit-gangal Oct 29, 2020
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
6 changes: 6 additions & 0 deletions go/sqltypes/value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,12 @@ func (v Value) IsBinary() bool {
return IsBinary(v.typ)
}

// IsDateTime returns true if Value is datetime.
func (v Value) IsDateTime() bool {
dt := int(querypb.Type_DATETIME)
return int(v.typ)&dt == dt
}

// MarshalJSON should only be used for testing.
// It's not a complete implementation.
func (v Value) MarshalJSON() ([]byte, error) {
Expand Down
6 changes: 6 additions & 0 deletions go/test/endtoend/vtgate/reservedconn/udv_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ func TestSetUDV(t *testing.T) {
}, {
query: "select id, val1 from test where val1 = @tablet",
expectedRows: `[[INT64(42) VARCHAR("foobar")]]`, rowsAffected: 1,
}, {
query: "set @foo = now(), @bar = now()",
expectedRows: `[]`, rowsAffected: 0,
}, {
query: "select @foo = @bar",
expectedRows: `[[INT64(1)]]`, rowsAffected: 1,
}}

conn, err := mysql.Connect(ctx, &vtParams)
Expand Down
36 changes: 35 additions & 1 deletion go/vt/vtgate/engine/set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func TestSetTable(t *testing.T) {
expectedQueryLog []string
expectedWarning []*querypb.QueryWarning
expectedError string
input Primitive
}

tests := []testCase{
Expand All @@ -91,6 +92,36 @@ func TestSetTable(t *testing.T) {
`UDV set with (x,INT64(42))`,
},
},
{
testName: "udv with input",
setOps: []SetOp{
&UserDefinedVariable{
Name: "x",
Expr: evalengine.NewColumn(0),
},
},
qr: []*sqltypes.Result{sqltypes.MakeTestResult(
sqltypes.MakeTestFields(
"col0",
"datetime",
),
"2020-10-28",
)},
expectedQueryLog: []string{
`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
`ExecuteMultiShard ks.-20: select now() from dual {} false false`,
`UDV set with (x,DATETIME("2020-10-28"))`,
},
input: &Send{
Keyspace: &vindexes.Keyspace{
Name: "ks",
Sharded: true,
},
TargetDestination: key.DestinationAnyShard{},
Query: "select now() from dual",
SingleShardOnly: true,
},
},
{
testName: "sysvar ignore",
setOps: []SetOp{
Expand Down Expand Up @@ -259,9 +290,12 @@ func TestSetTable(t *testing.T) {

for _, tc := range tests {
t.Run(tc.testName, func(t *testing.T) {
if tc.input == nil {
tc.input = &SingleRow{}
}
set := &Set{
Ops: tc.setOps,
Input: &SingleRow{},
Input: tc.input,
}
vc := &loggingVCursor{
shards: []string{"-20", "20-"},
Expand Down
4 changes: 3 additions & 1 deletion go/vt/vtgate/evalengine/arithmetic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ func newEvalResult(v sqltypes.Value) (EvalResult, error) {
return EvalResult{}, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%v", err)
}
return EvalResult{fval: fval, typ: sqltypes.Float64}, nil
case v.IsDateTime():
return EvalResult{typ: sqltypes.Datetime, bytes: raw}, nil
}
return EvalResult{}, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "this should not be reached. got %s", v.String())
}
Expand Down Expand Up @@ -752,7 +754,7 @@ func castFromNumeric(v EvalResult, resultType querypb.Type) sqltypes.Value {
}
return sqltypes.MakeTrusted(resultType, strconv.AppendFloat(nil, v.fval, format, -1, 64))
}
case resultType == sqltypes.VarChar || resultType == sqltypes.VarBinary || resultType == sqltypes.Binary || resultType == sqltypes.Text:
case resultType == sqltypes.VarChar || resultType == sqltypes.VarBinary || resultType == sqltypes.Binary || resultType == sqltypes.Text || resultType == sqltypes.Datetime:
return sqltypes.MakeTrusted(resultType, v.bytes)
}
return sqltypes.NULL
Expand Down
7 changes: 0 additions & 7 deletions go/vt/vtgate/planbuilder/expression_converter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ import (
"fmt"
"strings"

vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vterrors"
"vitess.io/vitess/go/vt/vtgate/engine"
"vitess.io/vitess/go/vt/vtgate/evalengine"
)
Expand Down Expand Up @@ -87,11 +85,6 @@ func (ec *expressionConverter) convert(astExpr sqlparser.Expr, boolean, identifi
if err != sqlparser.ErrExprNotSupported {
return nil, err
}
// We have an expression that we can't handle at the vtgate level
if !expressionOkToDelegateToTablet(astExpr) {
// Uh-oh - this expression is not even safe to delegate to the tablet. Give up.
return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "expression not supported for SET: %s", sqlparser.String(astExpr))
}
evalExpr = &evalengine.Column{Offset: len(ec.tabletExpressions)}
ec.tabletExpressions = append(ec.tabletExpressions, astExpr)
}
Expand Down
154 changes: 0 additions & 154 deletions go/vt/vtgate/planbuilder/set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,26 +145,6 @@ func planSysVarCheckIgnore(expr *sqlparser.SetExpr, schema ContextVSchema, boole
}, nil
}

func expressionOkToDelegateToTablet(e sqlparser.Expr) bool {
valid := true
sqlparser.Rewrite(e, nil, func(cursor *sqlparser.Cursor) bool {
switch n := cursor.Node().(type) {
case *sqlparser.Subquery, *sqlparser.TimestampFuncExpr, *sqlparser.CurTimeFuncExpr:
valid = false
return false
case *sqlparser.FuncExpr:
_, ok := validFuncs[n.Name.Lowered()]
valid = ok
return ok
case *sqlparser.ColName:
valid = n.Name.AtCount() == 2
return false
}
return true
})
return valid
}

func buildSetOpReservedConn(s setting) planFunc {
return func(expr *sqlparser.SetExpr, vschema ContextVSchema, _ *expressionConverter) (engine.SetOp, error) {
if !vschema.SysVarSetEnabled() {
Expand Down Expand Up @@ -257,137 +237,3 @@ func extractValue(expr *sqlparser.SetExpr, boolean bool) (string, error) {

return sqlparser.String(expr.Expr), nil
}

// whitelist of functions knows to be safe to pass through to mysql for evaluation
// this list tries to not include functions that might return different results on different tablets
var validFuncs = map[string]interface{}{
"if": nil,
"ifnull": nil,
"nullif": nil,
"abs": nil,
"acos": nil,
"asin": nil,
"atan2": nil,
"atan": nil,
"ceil": nil,
"ceiling": nil,
"conv": nil,
"cos": nil,
"cot": nil,
"crc32": nil,
"degrees": nil,
"div": nil,
"exp": nil,
"floor": nil,
"ln": nil,
"log": nil,
"log10": nil,
"log2": nil,
"mod": nil,
"pi": nil,
"pow": nil,
"power": nil,
"radians": nil,
"rand": nil,
"round": nil,
"sign": nil,
"sin": nil,
"sqrt": nil,
"tan": nil,
"truncate": nil,
"adddate": nil,
"addtime": nil,
"convert_tz": nil,
"date": nil,
"date_add": nil,
"date_format": nil,
"date_sub": nil,
"datediff": nil,
"day": nil,
"dayname": nil,
"dayofmonth": nil,
"dayofweek": nil,
"dayofyear": nil,
"extract": nil,
"from_days": nil,
"from_unixtime": nil,
"get_format": nil,
"hour": nil,
"last_day": nil,
"makedate": nil,
"maketime": nil,
"microsecond": nil,
"minute": nil,
"month": nil,
"monthname": nil,
"period_add": nil,
"period_diff": nil,
"quarter": nil,
"sec_to_time": nil,
"second": nil,
"str_to_date": nil,
"subdate": nil,
"subtime": nil,
"time_format": nil,
"time_to_sec": nil,
"timediff": nil,
"timestampadd": nil,
"timestampdiff": nil,
"to_days": nil,
"to_seconds": nil,
"week": nil,
"weekday": nil,
"weekofyear": nil,
"year": nil,
"yearweek": nil,
"ascii": nil,
"bin": nil,
"bit_length": nil,
"char": nil,
"char_length": nil,
"character_length": nil,
"concat": nil,
"concat_ws": nil,
"elt": nil,
"export_set": nil,
"field": nil,
"find_in_set": nil,
"format": nil,
"from_base64": nil,
"hex": nil,
"insert": nil,
"instr": nil,
"lcase": nil,
"left": nil,
"length": nil,
"load_file": nil,
"locate": nil,
"lower": nil,
"lpad": nil,
"ltrim": nil,
"make_set": nil,
"mid": nil,
"oct": nil,
"octet_length": nil,
"ord": nil,
"position": nil,
"quote": nil,
"repeat": nil,
"replace": nil,
"reverse": nil,
"right": nil,
"rpad": nil,
"rtrim": nil,
"soundex": nil,
"space": nil,
"strcmp": nil,
"substr": nil,
"substring": nil,
"substring_index": nil,
"to_base64": nil,
"trim": nil,
"ucase": nil,
"unhex": nil,
"upper": nil,
"weight_string": nil,
}
52 changes: 0 additions & 52 deletions go/vt/vtgate/planbuilder/set_test.go
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions go/vt/vtgate/planbuilder/testdata/set_cases.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@
}
}

# only allow whitelisted functions
"set @foo = BAD_FUNC()"
"expression not supported for SET: BAD_FUNC()"

# single sysvar cases
"SET sql_mode = 'STRICT_ALL_TABLES,NO_AUTO_VALUE_ON_ZERO'"
{
Expand Down