From f427e6cc690069042bda6af9b9e2315285267e0d Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 8 Apr 2020 18:28:39 +0530 Subject: [PATCH 1/5] set plan for user defined variables Signed-off-by: Harshit Gangal --- go/vt/sqlparser/ast.go | 7 +- go/vt/sqlparser/ast_funcs.go | 11 +-- go/vt/vtgate/engine/send.go | 7 +- go/vt/vtgate/engine/set.go | 82 +++++++++++++++++++ go/vt/vtgate/planbuilder/builder.go | 4 +- go/vt/vtgate/planbuilder/plan_test.go | 1 + go/vt/vtgate/planbuilder/set.go | 51 ++++++++++++ .../vtgate/planbuilder/testdata/set_cases.txt | 37 +++++++++ .../testdata/unsupported_cases.txt | 8 ++ 9 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 go/vt/vtgate/engine/set.go create mode 100644 go/vt/vtgate/planbuilder/set.go create mode 100644 go/vt/vtgate/planbuilder/testdata/set_cases.txt diff --git a/go/vt/sqlparser/ast.go b/go/vt/sqlparser/ast.go index a1340a6566b..09c85e8ce93 100644 --- a/go/vt/sqlparser/ast.go +++ b/go/vt/sqlparser/ast.go @@ -797,7 +797,7 @@ type ColIdent struct { // last field in the struct. _ [0]struct{ _ []byte } val, lowered string - at atCount + at AtCount } // TableIdent is a case sensitive SQL identifier. It will be escaped with @@ -1750,3 +1750,8 @@ func (node ColIdent) Format(buf *TrackedBuffer) { func (node TableIdent) Format(buf *TrackedBuffer) { formatID(buf, node.v, strings.ToLower(node.v), NoAt) } + +// AtCount return the '@' count present in ColIdent Name +func (node ColIdent) AtCount() AtCount { + return node.at +} diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index dada62f3125..b9ed7137435 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -530,7 +530,7 @@ func NewColIdent(str string) ColIdent { } // NewColIdentWithAt makes a new ColIdent. -func NewColIdentWithAt(str string, at atCount) ColIdent { +func NewColIdentWithAt(str string, at AtCount) ColIdent { return ColIdent{ val: str, at: at, @@ -639,7 +639,7 @@ func (node *TableIdent) UnmarshalJSON(b []byte) error { return nil } -func containEscapableChars(s string, at atCount) bool { +func containEscapableChars(s string, at AtCount) bool { isDbSystemVariable := at != NoAt for i, c := range s { @@ -660,7 +660,7 @@ func isKeyword(s string) bool { return isKeyword } -func formatID(buf *TrackedBuffer, original, lowered string, at atCount) { +func formatID(buf *TrackedBuffer, original, lowered string, at AtCount) { if containEscapableChars(original, at) || isKeyword(lowered) { writeEscapedString(buf, original) } else { @@ -755,11 +755,12 @@ func (node *Union) SetLimit(limit *Limit) { node.Limit = limit } -type atCount int +// AtCount represents the '@' count in ColIdent +type AtCount int const ( // NoAt represents no @ - NoAt atCount = iota + NoAt AtCount = iota // SingleAt represents @ SingleAt // DoubleAt represnts @@ diff --git a/go/vt/vtgate/engine/send.go b/go/vt/vtgate/engine/send.go index 9c5a0c8ef30..8eab3a8d067 100644 --- a/go/vt/vtgate/engine/send.go +++ b/go/vt/vtgate/engine/send.go @@ -19,7 +19,6 @@ package engine import ( "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" - "vitess.io/vitess/go/vt/proto/query" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/vindexes" @@ -74,7 +73,7 @@ func (s *Send) GetTableName() string { } // Execute implements Primitive interface -func (s *Send) Execute(vcursor VCursor, bindVars map[string]*query.BindVariable, _ bool) (*sqltypes.Result, error) { +func (s *Send) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, _ bool) (*sqltypes.Result, error) { rss, _, err := vcursor.ResolveDestinations(s.Keyspace.Name, nil, []key.Destination{s.TargetDestination}) if err != nil { return nil, vterrors.Wrap(err, "sendExecute") @@ -111,12 +110,12 @@ func (s *Send) Execute(vcursor VCursor, bindVars map[string]*query.BindVariable, } // StreamExecute implements Primitive interface -func (s *Send) StreamExecute(vcursor VCursor, bindVars map[string]*query.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error { +func (s *Send) StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error { return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "not reachable") // TODO: systay - this should work } // GetFields implements Primitive interface -func (s *Send) GetFields(vcursor VCursor, bindVars map[string]*query.BindVariable) (*sqltypes.Result, error) { +func (s *Send) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "not reachable") } diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go new file mode 100644 index 00000000000..44dd141fc18 --- /dev/null +++ b/go/vt/vtgate/engine/set.go @@ -0,0 +1,82 @@ +package engine + +import ( + "vitess.io/vitess/go/sqltypes" + querypb "vitess.io/vitess/go/vt/proto/query" +) + +type ( + // Set contains the instructions to perform set. + Set struct { + Ops []SetOp + noTxNeeded + noInputs + } + + // SetOp is an interface that different type of set operations implements. + SetOp interface { + Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable) error + VariableName() string + } + + // UserDefinedVariable implements the SetOp interface to execute user defined variables. + UserDefinedVariable struct { + Name string + Value sqltypes.PlanValue + } +) + +var _ Primitive = (*Set)(nil) + +//RouteType implements the Primitive interface method. +func (s *Set) RouteType() string { + return "Set" +} + +//GetKeyspaceName implements the Primitive interface method. +func (s *Set) GetKeyspaceName() string { + return "" +} + +//GetTableName implements the Primitive interface method. +func (s *Set) GetTableName() string { + return "" +} + +//Execute implements the Primitive interface method. +func (s *Set) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) { + panic("implement me") +} + +//StreamExecute implements the Primitive interface method. +func (s *Set) StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error { + panic("implement me") +} + +//GetFields implements the Primitive interface method. +func (s *Set) GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { + panic("implement me") +} + +func (s *Set) description() PrimitiveDescription { + other := map[string]interface{}{ + "Ops": s.Ops, + } + return PrimitiveDescription{ + OperatorType: "Set", + Variant: "", + Other: other, + } +} + +var _ SetOp = (*UserDefinedVariable)(nil) + +//VariableName implements the SetOp interface method. +func (u *UserDefinedVariable) VariableName() string { + return u.Name +} + +//Execute implements the SetOp interface method. +func (u *UserDefinedVariable) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable) error { + panic("implement me") +} diff --git a/go/vt/vtgate/planbuilder/builder.go b/go/vt/vtgate/planbuilder/builder.go index ac6a3a79dc6..091235bd2b7 100644 --- a/go/vt/vtgate/planbuilder/builder.go +++ b/go/vt/vtgate/planbuilder/builder.go @@ -303,7 +303,9 @@ func BuildFromStmt(query string, stmt sqlparser.Statement, vschema ContextVSchem instruction, err = buildUsePlan(stmt, vschema) case *sqlparser.OtherRead, *sqlparser.OtherAdmin: instruction, err = buildOtherReadAndAdmin(query, vschema) - case *sqlparser.Set, *sqlparser.Show, *sqlparser.DBDDL: + case *sqlparser.Set: + instruction, err = buildSetPlan(query, stmt, vschema) + case *sqlparser.Show, *sqlparser.DBDDL: return nil, ErrPlanNotSupported case *sqlparser.Begin, *sqlparser.Commit, *sqlparser.Rollback: // Empty by design. Not executed by a plan diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index ae445a8ed3d..509e14b37f9 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -169,6 +169,7 @@ func TestPlan(t *testing.T) { testFile(t, "wireup_cases.txt", testOutputTempDir, vschemaWrapper) testFile(t, "memory_sort_cases.txt", testOutputTempDir, vschemaWrapper) testFile(t, "use_cases.txt", testOutputTempDir, vschemaWrapper) + testFile(t, "set_cases.txt", testOutputTempDir, vschemaWrapper) } func TestOne(t *testing.T) { diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go new file mode 100644 index 00000000000..22e46e65a88 --- /dev/null +++ b/go/vt/vtgate/planbuilder/set.go @@ -0,0 +1,51 @@ +package planbuilder + +import ( + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/vtgate/engine" +) + +func buildSetPlan(sql string, stmt *sqlparser.Set, vschema ContextVSchema) (engine.Primitive, error) { + //keyValues, scope, err := sqlparser.ExtractSetValues(sql) + //if err != nil { + // return nil, err + //} + //if scope == sqlparser.GlobalStr { + // return nil, vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported in set: global") + //} + var setOps []engine.SetOp + + for _, expr := range stmt.Exprs { + switch expr.Name.AtCount() { + case sqlparser.SingleAt: + default: + return nil, ErrPlanNotSupported + } + pv, err := sqlparser.NewPlanValue(expr.Expr) + if err != nil { + return nil, err + } + setOps = append(setOps, &engine.UserDefinedVariable{ + Name: expr.Name.Lowered(), + Value: pv, + }) + } + //for key, value := range keyValues { + // switch key.Scope { + // case sqlparser.VariableStr: + // setOps = append(setOps, &engine.UserDefinedVariable{ + // Value: sqltypes.PlanValue{ + // Key: key.Key, + // Value: value, + // ListKey: "", + // Values: nil, + // }, + // }) + // default: + // return nil, ErrPlanNotSupported + // } + //} + return &engine.Set{ + Ops: setOps, + }, nil +} diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_cases.txt new file mode 100644 index 00000000000..2206644e2c6 --- /dev/null +++ b/go/vt/vtgate/planbuilder/testdata/set_cases.txt @@ -0,0 +1,37 @@ +# set single user defined variable +"set @foo = 42" +{ + "QueryType": "SET", + "Original": "set @foo = 42", + "Instructions": { + "OperatorType": "Set", + "Variant": "", + "Ops": [ + { + "Name": "foo", + "Value": 42 + } + ] + } +} + +# set multi user defined variable +"set @foo = 42, @bar = @foo" +{ + "QueryType": "SET", + "Original": "set @foo = 42, @bar = @foo", + "Instructions": { + "OperatorType": "Set", + "Variant": "", + "Ops": [ + { + "Name": "foo", + "Value": 42 + }, + { + "Name": "bar", + "Value": ":__vtudvfoo" + } + ] + } +} diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt index f61672a9c14..b9173fb4094 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt @@ -6,6 +6,14 @@ "set a=1" "plan building not supported" +# set multi user defined variable with complex expression +"set @foo = 42, @bar = @foo + 1" +"expression is too complex ':__vtudvfoo + 1'" + +# set user defined and system variable +"set @foo = 42, @bar = @foo, @@sql_mode = @@sql_mode" +"plan building not supported" + # SHOW "show create database" "plan building not supported" From 2cba596903f0d66dceae49ed199b56d79ac7ca4e Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 9 Apr 2020 10:54:03 +0530 Subject: [PATCH 2/5] set plan engine to execute the primitive Signed-off-by: Harshit Gangal --- go/vt/sqlparser/analyzer.go | 2 ++ go/vt/vtgate/engine/fake_vcursor_test.go | 9 +++++++ go/vt/vtgate/engine/primitive.go | 3 +++ go/vt/vtgate/engine/set.go | 18 ++++++++++--- go/vt/vtgate/executor_select_test.go | 10 +++---- go/vt/vtgate/executor_set_test.go | 17 +++++------- go/vt/vtgate/plan_executor_select_test.go | 10 +++---- go/vt/vtgate/planbuilder/set.go | 26 ++----------------- .../vtgate/planbuilder/testdata/set_cases.txt | 6 ++--- go/vt/vtgate/vcursor_impl.go | 9 +++++++ 10 files changed, 59 insertions(+), 51 deletions(-) diff --git a/go/vt/sqlparser/analyzer.go b/go/vt/sqlparser/analyzer.go index 503d26a240b..3469edcd0c2 100644 --- a/go/vt/sqlparser/analyzer.go +++ b/go/vt/sqlparser/analyzer.go @@ -332,6 +332,8 @@ func NewPlanValue(node Expr) (sqltypes.PlanValue, error) { return sqltypes.PlanValue{}, err } return sqltypes.PlanValue{Value: n}, nil + case FloatVal: + return sqltypes.PlanValue{Value: sqltypes.MakeTrusted(sqltypes.Float64, node.Val)}, nil case StrVal: return sqltypes.PlanValue{Value: sqltypes.MakeTrusted(sqltypes.VarBinary, node.Val)}, nil case HexVal: diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go index 20290b08863..2b7acc6d809 100644 --- a/go/vt/vtgate/engine/fake_vcursor_test.go +++ b/go/vt/vtgate/engine/fake_vcursor_test.go @@ -47,6 +47,10 @@ var _ VCursor = (*noopVCursor)(nil) type noopVCursor struct { } +func (t noopVCursor) SetUDV(key string, value interface{}) error { + panic("implement me") +} + func (t noopVCursor) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error { panic("implement me") } @@ -123,6 +127,11 @@ type loggingVCursor struct { log []string } +func (f *loggingVCursor) SetUDV(key string, value interface{}) error { + f.log = append(f.log, fmt.Sprintf("UDV set with (%s,%v)", key, value)) + return nil +} + func (f *loggingVCursor) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error { panic("implement me") } diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go index 0ce5ed276bc..3cecd6dd5a4 100644 --- a/go/vt/vtgate/engine/primitive.go +++ b/go/vt/vtgate/engine/primitive.go @@ -72,7 +72,10 @@ type VCursor interface { // Resolver methods, from key.Destination to srvtopo.ResolvedShard. // Will replace all of the Topo functions. ResolveDestinations(keyspace string, ids []*querypb.Value, destinations []key.Destination) ([]*srvtopo.ResolvedShard, [][]*querypb.Value, error) + SetTarget(target string) error + SetUDV(key string, value interface{}) error + ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error } diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go index 44dd141fc18..d8eb28f64ec 100644 --- a/go/vt/vtgate/engine/set.go +++ b/go/vt/vtgate/engine/set.go @@ -21,8 +21,8 @@ type ( // UserDefinedVariable implements the SetOp interface to execute user defined variables. UserDefinedVariable struct { - Name string - Value sqltypes.PlanValue + Name string + PlanValue sqltypes.PlanValue } ) @@ -45,7 +45,13 @@ func (s *Set) GetTableName() string { //Execute implements the Primitive interface method. func (s *Set) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error) { - panic("implement me") + for _, setOp := range s.Ops { + err := setOp.Execute(vcursor, bindVars) + if err != nil { + return nil, err + } + } + return &sqltypes.Result{}, nil } //StreamExecute implements the Primitive interface method. @@ -78,5 +84,9 @@ func (u *UserDefinedVariable) VariableName() string { //Execute implements the SetOp interface method. func (u *UserDefinedVariable) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable) error { - panic("implement me") + value, err := u.PlanValue.ResolveValue(bindVars) + if err != nil { + return err + } + return vcursor.SetUDV(u.Name, value) } diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go index 86b4ffdaf59..cac773aaaad 100644 --- a/go/vt/vtgate/executor_select_test.go +++ b/go/vt/vtgate/executor_select_test.go @@ -250,7 +250,7 @@ func TestSelectUserDefindVariable(t *testing.T) { require.NoError(t, err) wantQueries := []*querypb.BoundQuery{{ Sql: "select :__vtudvfoo as `@foo` from dual", - BindVariables: map[string]*querypb.BindVariable{"__vtudvfoo": sqltypes.StringBindVariable("bar")}, + BindVariables: map[string]*querypb.BindVariable{"__vtudvfoo": sqltypes.BytesBindVariable([]byte("bar"))}, }} assert.Equal(t, wantQueries, sbc1.Queries) @@ -409,15 +409,15 @@ func TestSelectBindvars(t *testing.T) { // Test with StringBindVariable sql = "select id from user where name in (:name1, :name2)" _, err = executorExec(executor, sql, map[string]*querypb.BindVariable{ - "name1": sqltypes.StringBindVariable("foo1"), - "name2": sqltypes.StringBindVariable("foo2"), + "name1": sqltypes.BytesBindVariable([]byte("foo1")), + "name2": sqltypes.BytesBindVariable([]byte("foo2")), }) require.NoError(t, err) wantQueries = []*querypb.BoundQuery{{ Sql: "select id from user where name in ::__vals", BindVariables: map[string]*querypb.BindVariable{ - "name1": sqltypes.StringBindVariable("foo1"), - "name2": sqltypes.StringBindVariable("foo2"), + "name1": sqltypes.BytesBindVariable([]byte("foo1")), + "name2": sqltypes.BytesBindVariable([]byte("foo2")), "__vals": sqltypes.TestBindVariable([]interface{}{"foo1", "foo2"}), }, }} diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index 1fa5f8605a4..72562be7d2e 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -19,11 +19,12 @@ package vtgate import ( "testing" + "vitess.io/vitess/go/test/utils" + "vitess.io/vitess/go/vt/vterrors" "context" - "github.com/golang/protobuf/proto" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/vtgate/vschemaacl" @@ -262,21 +263,17 @@ func TestExecutorSet(t *testing.T) { in: "set @foo = 2", out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo"}, []interface{}{2}), Autocommit: true}, }, { - in: "set @foo = 2.0, @bar = 'baz'", - out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo", "bar"}, []interface{}{2.0, "baz"}), Autocommit: true}, + in: "set @foo = 2.1, @bar = 'baz'", + out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo", "bar"}, []interface{}{2.1, "baz"}), Autocommit: true}, }} for _, tcase := range testcases { t.Run(tcase.in, func(t *testing.T) { session := NewSafeSession(&vtgatepb.Session{Autocommit: true}) _, err := executor.Execute(context.Background(), "TestExecute", session, tcase.in, nil) if err != nil { - if err.Error() != tcase.err { - t.Errorf("%s error: %v, want %s", tcase.in, err, tcase.err) - } - return - } - if !proto.Equal(session.Session, tcase.out) { - t.Errorf("%s: %v, want %s", tcase.in, session.Session, tcase.out) + require.EqualError(t, err, tcase.err) + } else { + utils.MustMatch(t, tcase.out, session.Session, "session output was not as expected") } }) } diff --git a/go/vt/vtgate/plan_executor_select_test.go b/go/vt/vtgate/plan_executor_select_test.go index 5d234e5da07..72a2426c390 100644 --- a/go/vt/vtgate/plan_executor_select_test.go +++ b/go/vt/vtgate/plan_executor_select_test.go @@ -249,7 +249,7 @@ func TestPlanSelectUserDefindVariable(t *testing.T) { require.NoError(t, err) wantQueries := []*querypb.BoundQuery{{ Sql: "select :__vtudvfoo as `@foo` from dual", - BindVariables: map[string]*querypb.BindVariable{"__vtudvfoo": sqltypes.StringBindVariable("bar")}, + BindVariables: map[string]*querypb.BindVariable{"__vtudvfoo": sqltypes.BytesBindVariable([]byte("bar"))}, }} assert.Equal(t, wantQueries, sbc1.Queries) @@ -408,15 +408,15 @@ func TestPlanSelectBindvars(t *testing.T) { // Test with StringBindVariable sql = "select id from user where name in (:name1, :name2)" _, err = executorExec(executor, sql, map[string]*querypb.BindVariable{ - "name1": sqltypes.StringBindVariable("foo1"), - "name2": sqltypes.StringBindVariable("foo2"), + "name1": sqltypes.BytesBindVariable([]byte("foo1")), + "name2": sqltypes.BytesBindVariable([]byte("foo2")), }) require.NoError(t, err) wantQueries = []*querypb.BoundQuery{{ Sql: "select id from user where name in ::__vals", BindVariables: map[string]*querypb.BindVariable{ - "name1": sqltypes.StringBindVariable("foo1"), - "name2": sqltypes.StringBindVariable("foo2"), + "name1": sqltypes.BytesBindVariable([]byte("foo1")), + "name2": sqltypes.BytesBindVariable([]byte("foo2")), "__vals": sqltypes.TestBindVariable([]interface{}{"foo1", "foo2"}), }, }} diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go index 22e46e65a88..7c7d8ac136a 100644 --- a/go/vt/vtgate/planbuilder/set.go +++ b/go/vt/vtgate/planbuilder/set.go @@ -6,13 +6,6 @@ import ( ) func buildSetPlan(sql string, stmt *sqlparser.Set, vschema ContextVSchema) (engine.Primitive, error) { - //keyValues, scope, err := sqlparser.ExtractSetValues(sql) - //if err != nil { - // return nil, err - //} - //if scope == sqlparser.GlobalStr { - // return nil, vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported in set: global") - //} var setOps []engine.SetOp for _, expr := range stmt.Exprs { @@ -26,25 +19,10 @@ func buildSetPlan(sql string, stmt *sqlparser.Set, vschema ContextVSchema) (engi return nil, err } setOps = append(setOps, &engine.UserDefinedVariable{ - Name: expr.Name.Lowered(), - Value: pv, + Name: expr.Name.Lowered(), + PlanValue: pv, }) } - //for key, value := range keyValues { - // switch key.Scope { - // case sqlparser.VariableStr: - // setOps = append(setOps, &engine.UserDefinedVariable{ - // Value: sqltypes.PlanValue{ - // Key: key.Key, - // Value: value, - // ListKey: "", - // Values: nil, - // }, - // }) - // default: - // return nil, ErrPlanNotSupported - // } - //} return &engine.Set{ Ops: setOps, }, nil diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_cases.txt index 2206644e2c6..99d4f72e920 100644 --- a/go/vt/vtgate/planbuilder/testdata/set_cases.txt +++ b/go/vt/vtgate/planbuilder/testdata/set_cases.txt @@ -9,7 +9,7 @@ "Ops": [ { "Name": "foo", - "Value": 42 + "PlanValue": 42 } ] } @@ -26,11 +26,11 @@ "Ops": [ { "Name": "foo", - "Value": 42 + "PlanValue": 42 }, { "Name": "bar", - "Value": ":__vtudvfoo" + "PlanValue": ":__vtudvfoo" } ] } diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go index 24042a2dadd..02a167846ed 100644 --- a/go/vt/vtgate/vcursor_impl.go +++ b/go/vt/vtgate/vcursor_impl.go @@ -328,6 +328,15 @@ func (vc *vcursorImpl) SetTarget(target string) error { return nil } +func (vc *vcursorImpl) SetUDV(key string, value interface{}) error { + bindValue, err := sqltypes.BuildBindVariable(value) + if err != nil { + return err + } + vc.safeSession.SetUserDefinedVariable(key, bindValue) + return nil +} + // Destination implements the ContextVSchema interface func (vc *vcursorImpl) Destination() key.Destination { return vc.destination From 1fc7b80cc8f0fedf1087271cfafd0755dadcfecf Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 9 Apr 2020 16:36:16 +0530 Subject: [PATCH 3/5] fix test: changed varchar to varbinary in bindvars Signed-off-by: Harshit Gangal --- go/vt/vtgate/executor_set_test.go | 12 - go/vt/vtgate/plan_executor_set_test.go | 360 ++++++++++++++++++ .../messager/message_manager_test.go | 10 +- 3 files changed, 364 insertions(+), 18 deletions(-) create mode 100644 go/vt/vtgate/plan_executor_set_test.go diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index 72562be7d2e..5dc1423de38 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -36,18 +36,6 @@ import ( "github.com/stretchr/testify/require" ) -func createMap(keys []string, values []interface{}) map[string]*querypb.BindVariable { - result := make(map[string]*querypb.BindVariable) - for i, key := range keys { - variable, err := sqltypes.BuildBindVariable(values[i]) - if err != nil { - panic(err) - } - result[key] = variable - } - return result -} - func TestExecutorSet(t *testing.T) { executor, _, _, _ := createExecutorEnv() diff --git a/go/vt/vtgate/plan_executor_set_test.go b/go/vt/vtgate/plan_executor_set_test.go new file mode 100644 index 00000000000..13cc5c7fcd1 --- /dev/null +++ b/go/vt/vtgate/plan_executor_set_test.go @@ -0,0 +1,360 @@ +/* +Copyright 2019 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vtgate + +import ( + "testing" + + "vitess.io/vitess/go/test/utils" + + "vitess.io/vitess/go/vt/vterrors" + + "context" + + "vitess.io/vitess/go/sqltypes" + "vitess.io/vitess/go/vt/vtgate/vschemaacl" + + querypb "vitess.io/vitess/go/vt/proto/query" + vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createMap(keys []string, values []interface{}) map[string]*querypb.BindVariable { + result := make(map[string]*querypb.BindVariable) + for i, key := range keys { + variable, err := sqltypes.BuildBindVariable(values[i]) + if err != nil { + panic(err) + } + result[key] = variable + } + return result +} + +func TestPlanExecutorSetUDV(t *testing.T) { + executor, _, _, _ := createExecutorEnvUsing(planAllTheThings) + + testcases := []struct { + in string + out *vtgatepb.Session + err string + }{{ + in: "set @foo = 'bar'", + out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo"}, []interface{}{"bar"}), Autocommit: true}, + }, { + in: "set @foo = 2", + out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo"}, []interface{}{2}), Autocommit: true}, + }, { + in: "set @foo = 2.1, @bar = 'baz'", + out: &vtgatepb.Session{UserDefinedVariables: createMap([]string{"foo", "bar"}, []interface{}{2.1, "baz"}), Autocommit: true}, + }} + for _, tcase := range testcases { + t.Run(tcase.in, func(t *testing.T) { + session := NewSafeSession(&vtgatepb.Session{Autocommit: true}) + _, err := executor.Execute(context.Background(), "TestExecute", session, tcase.in, nil) + if err != nil { + require.EqualError(t, err, tcase.err) + } else { + utils.MustMatch(t, tcase.out, session.Session, "session output was not as expected") + } + }) + } +} + +func TestPlanExecutorSetSystemVar(t *testing.T) { + t.Skip("Planning for system variables is not done yet") + executor, _, _, _ := createExecutorEnvUsing(planAllTheThings) + + testcases := []struct { + in string + out *vtgatepb.Session + err string + }{{ + in: "set autocommit = 1", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set @@autocommit = true", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set @@session.autocommit = true", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set @@session.`autocommit` = true", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set @@session.'autocommit' = true", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set @@session.\"autocommit\" = true", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set autocommit = true", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set autocommit = on", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set autocommit = ON", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set autocommit = 'on'", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set autocommit = `on`", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set autocommit = \"on\"", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set autocommit = false", + out: &vtgatepb.Session{}, + }, { + in: "set autocommit = off", + out: &vtgatepb.Session{}, + }, { + in: "set autocommit = OFF", + out: &vtgatepb.Session{}, + }, { + in: "set AUTOCOMMIT = 0", + out: &vtgatepb.Session{}, + }, { + in: "set AUTOCOMMIT = 'aa'", + err: "unexpected value for autocommit: aa", + }, { + in: "set autocommit = 2", + err: "unexpected value for autocommit: 2", + }, { + in: "set client_found_rows = 1", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{ClientFoundRows: true}}, + }, { + in: "set client_found_rows = true", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{ClientFoundRows: true}}, + }, { + in: "set client_found_rows = 0", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}}, + }, { + in: "set client_found_rows = false", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}}, + }, { + in: "set @@global.client_found_rows = 1", + err: "unsupported in set: global", + }, { + in: "set global client_found_rows = 1", + err: "unsupported in set: global", + }, { + in: "set global @@session.client_found_rows = 1", + err: "unsupported in set: mixed using of variable scope", + }, { + in: "set client_found_rows = 'aa'", + err: "unexpected value type for client_found_rows: string", + }, { + in: "set client_found_rows = 2", + err: "unexpected value for client_found_rows: 2", + }, { + in: "set transaction_mode = 'unspecified'", + out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_UNSPECIFIED}, + }, { + in: "set transaction_mode = 'single'", + out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_SINGLE}, + }, { + in: "set transaction_mode = 'multi'", + out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_MULTI}, + }, { + in: "set transaction_mode = 'twopc'", + out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_TWOPC}, + }, { + in: "set transaction_mode = twopc", + out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_TWOPC}, + }, { + in: "set transaction_mode = 'aa'", + err: "invalid transaction_mode: aa", + }, { + in: "set transaction_mode = 1", + err: "unexpected value type for transaction_mode: int64", + }, { + in: "set workload = 'unspecified'", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{Workload: querypb.ExecuteOptions_UNSPECIFIED}}, + }, { + in: "set workload = 'oltp'", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{Workload: querypb.ExecuteOptions_OLTP}}, + }, { + in: "set workload = 'olap'", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{Workload: querypb.ExecuteOptions_OLAP}}, + }, { + in: "set workload = 'dba'", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{Workload: querypb.ExecuteOptions_DBA}}, + }, { + in: "set workload = 'aa'", + err: "invalid workload: aa", + }, { + in: "set workload = 1", + err: "unexpected value type for workload: int64", + }, { + in: "set transaction_mode = 'twopc', autocommit=1", + out: &vtgatepb.Session{Autocommit: true, TransactionMode: vtgatepb.TransactionMode_TWOPC}, + }, { + in: "set sql_select_limit = 5", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{SqlSelectLimit: 5}}, + }, { + in: "set sql_select_limit = DEFAULT", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{SqlSelectLimit: 0}}, + }, { + in: "set sql_select_limit = 'asdfasfd'", + err: "unexpected string value for sql_select_limit: asdfasfd", + }, { + in: "set autocommit = 1+1", + err: "invalid syntax: 1 + 1", + }, { + in: "set character_set_results=null", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set character_set_results='abcd'", + err: "disallowed value for character_set_results: abcd", + }, { + in: "set foo = 1", + err: "unsupported construct: set foo = 1", + }, { + in: "set names utf8", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set names ascii", + err: "unexpected value for charset/names: ascii", + }, { + in: "set charset utf8", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set character set default", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set character set ascii", + err: "unexpected value for charset/names: ascii", + }, { + in: "set net_write_timeout = 600", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set sql_mode = 'STRICT_ALL_TABLES'", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set net_read_timeout = 600", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set sql_quote_show_create = 1", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set foreign_key_checks = 0", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set unique_checks = 0", + out: &vtgatepb.Session{Autocommit: true}, + }, { + in: "set skip_query_plan_cache = 1", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{SkipQueryPlanCache: true}}, + }, { + in: "set skip_query_plan_cache = 0", + out: &vtgatepb.Session{Autocommit: true, Options: &querypb.ExecuteOptions{}}, + }, { + in: "set sql_auto_is_null = 0", + out: &vtgatepb.Session{Autocommit: true}, // no effect + }, { + in: "set sql_auto_is_null = 1", + err: "sql_auto_is_null is not currently supported", + }, { + in: "set tx_read_only = 2", + err: "unexpected value for tx_read_only: 2", + }, { + in: "set transaction_read_only = 2", + err: "unexpected value for transaction_read_only: 2", + }, { + in: "set tx_isolation = 'invalid'", + err: "unexpected value for tx_isolation: invalid", + }, { + in: "set sql_safe_updates = 2", + err: "unexpected value for sql_safe_updates: 2", + }} + for _, tcase := range testcases { + t.Run(tcase.in, func(t *testing.T) { + session := NewSafeSession(&vtgatepb.Session{Autocommit: true}) + _, err := executor.Execute(context.Background(), "TestExecute", session, tcase.in, nil) + if err != nil { + require.EqualError(t, err, tcase.err) + } else { + utils.MustMatch(t, tcase.out, session.Session, "session output was not as expected") + } + }) + } +} + +func TestPlanExecutorSetMetadata(t *testing.T) { + t.Skip("planning for vitess metadata is not supported yet") + executor, _, _, _ := createExecutorEnvUsing(planAllTheThings) + session := NewSafeSession(&vtgatepb.Session{TargetString: "@master", Autocommit: true}) + + set := "set @@vitess_metadata.app_keyspace_v1= '1'" + _, err := executor.Execute(context.Background(), "TestExecute", session, set, nil) + assert.Equalf(t, vtrpcpb.Code_PERMISSION_DENIED, vterrors.Code(err), "expected error %v, got error: %v", vtrpcpb.Code_PERMISSION_DENIED, err) + + *vschemaacl.AuthorizedDDLUsers = "%" + defer func() { + *vschemaacl.AuthorizedDDLUsers = "" + }() + + executor, _, _, _ = createExecutorEnvUsing(planAllTheThings) + session = NewSafeSession(&vtgatepb.Session{TargetString: "@master", Autocommit: true}) + + set = "set @@vitess_metadata.app_keyspace_v1= '1'" + _, err = executor.Execute(context.Background(), "TestExecute", session, set, nil) + assert.NoError(t, err, "%s error: %v", set, err) + + show := `show vitess_metadata variables like 'app\\_keyspace\\_v_'` + result, err := executor.Execute(context.Background(), "TestExecute", session, show, nil) + assert.NoError(t, err) + + want := "1" + got := string(result.Rows[0][1].ToString()) + assert.Equalf(t, want, got, "want migrations %s, result %s", want, got) + + // Update metadata + set = "set @@vitess_metadata.app_keyspace_v2='2'" + _, err = executor.Execute(context.Background(), "TestExecute", session, set, nil) + assert.NoError(t, err, "%s error: %v", set, err) + + show = `show vitess_metadata variables like 'app\\_keyspace\\_v%'` + gotqr, err := executor.Execute(context.Background(), "TestExecute", session, show, nil) + assert.NoError(t, err) + + wantqr := &sqltypes.Result{ + Fields: buildVarCharFields("Key", "Value"), + Rows: [][]sqltypes.Value{ + buildVarCharRow("app_keyspace_v1", "1"), + buildVarCharRow("app_keyspace_v2", "2"), + }, + RowsAffected: 2, + } + + assert.Equal(t, wantqr.Fields, gotqr.Fields) + assert.ElementsMatch(t, wantqr.Rows, gotqr.Rows) + + show = "show vitess_metadata variables" + gotqr, err = executor.Execute(context.Background(), "TestExecute", session, show, nil) + require.NoError(t, err) + + assert.Equal(t, wantqr.Fields, gotqr.Fields) + assert.ElementsMatch(t, wantqr.Rows, gotqr.Rows) +} diff --git a/go/vt/vttablet/tabletserver/messager/message_manager_test.go b/go/vt/vttablet/tabletserver/messager/message_manager_test.go index 54ac18b6e95..b9ee7a7fd73 100644 --- a/go/vt/vttablet/tabletserver/messager/message_manager_test.go +++ b/go/vt/vttablet/tabletserver/messager/message_manager_test.go @@ -26,6 +26,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/test/utils" + "github.com/stretchr/testify/assert" "golang.org/x/net/context" @@ -746,9 +748,7 @@ func TestMMGenerate(t *testing.T) { } gotids := bv["ids"] wantids := sqltypes.TestBindVariable([]interface{}{"1", "2"}) - if !reflect.DeepEqual(gotids, wantids) { - t.Errorf("gotid: %v, want %v", gotids, wantids) - } + utils.MustMatch(t, wantids, gotids, "did not match") query, bv = mm.GeneratePostponeQuery([]string{"1", "2"}) wantQuery = "update foo set time_next = :time_now+(:min_backoff< Date: Thu, 9 Apr 2020 17:36:04 +0530 Subject: [PATCH 4/5] add float test for plan value Signed-off-by: Harshit Gangal --- go/vt/sqlparser/analyzer_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/go/vt/sqlparser/analyzer_test.go b/go/vt/sqlparser/analyzer_test.go index f27ce3d9504..a0d6e731965 100644 --- a/go/vt/sqlparser/analyzer_test.go +++ b/go/vt/sqlparser/analyzer_test.go @@ -410,6 +410,12 @@ func TestNewPlanValue(t *testing.T) { }, { in: &NullVal{}, out: sqltypes.PlanValue{}, + }, { + in: &SQLVal{ + Type: FloatVal, + Val: []byte("2.1"), + }, + out: sqltypes.PlanValue{Value: sqltypes.NewFloat64(2.1)}, }} for _, tc := range tcases { got, err := NewPlanValue(tc.in) @@ -423,7 +429,7 @@ func TestNewPlanValue(t *testing.T) { t.Error(err) continue } - if !reflect.DeepEqual(got, tc.out) { + if !reflect.DeepEqual(tc.out, got) { t.Errorf("NewPlanValue(%s): %v, want %v", String(tc.in), got, tc.out) } } From 363fc5331f09a0ace57f7b3349484688cea47da5 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Tue, 14 Apr 2020 21:43:14 +0530 Subject: [PATCH 5/5] added license header Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/set.go | 16 ++++++++++++++++ go/vt/vtgate/planbuilder/set.go | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go index d8eb28f64ec..f4e2b025602 100644 --- a/go/vt/vtgate/engine/set.go +++ b/go/vt/vtgate/engine/set.go @@ -1,3 +1,19 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package engine import ( diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go index 7c7d8ac136a..4722a405fc5 100644 --- a/go/vt/vtgate/planbuilder/set.go +++ b/go/vt/vtgate/planbuilder/set.go @@ -1,3 +1,19 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package planbuilder import (