Repository: vitessio/vitess
Pull request #6073 (2020-04-16 07:51:13)

Add unit test for go/jsonutil/json.go
Signed-off-by: ZouYu <zouy.fnst@cn.fujitsu.com>


Patch:
diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go
index 72670d2d7a..3cecd6dd5a 100644
--- a/go/vt/vtgate/engine/primitive.go
+++ b/go/vt/vtgate/engine/primitive.go
@@ -42,101 +42,61 @@ const (
 	ListVarName = "__vals"
 )
 
-type (
-	// VCursor defines the interface the engine will use
-	// to execute routes.
-	VCursor interface {
-		// Context returns the context of the current request.
-		Context() context.Context
+// VCursor defines the interface the engine will use
+// to execute routes.
+type VCursor interface {
+	// Context returns the context of the current request.
+	Context() context.Context
 
-		// MaxMemoryRows returns the maxMemoryRows flag value.
-		MaxMemoryRows() int
+	// MaxMemoryRows returns the maxMemoryRows flag value.
+	MaxMemoryRows() int
 
-		// SetContextTimeout updates the context and sets a timeout.
-		SetContextTimeout(timeout time.Duration) context.CancelFunc
+	// SetContextTimeout updates the context and sets a timeout.
+	SetContextTimeout(timeout time.Duration) context.CancelFunc
 
-		// V3 functions.
-		Execute(method string, query string, bindvars map[string]*querypb.BindVariable, rollbackOnError bool, co vtgatepb.CommitOrder) (*sqltypes.Result, error)
-		AutocommitApproval() bool
+	// RecordWarning stores the given warning in the current session
+	RecordWarning(warning *querypb.QueryWarning)
 
-		// Shard-level functions.
-		ExecuteMultiShard(rss []*srvtopo.ResolvedShard, queries []*querypb.BoundQuery, rollbackOnError, canAutocommit bool) (*sqltypes.Result, []error)
-		ExecuteStandalone(query string, bindvars map[string]*querypb.BindVariable, rs *srvtopo.ResolvedShard) (*sqltypes.Result, error)
-		StreamExecuteMulti(query string, rss []*srvtopo.ResolvedShard, bindVars []map[string]*querypb.BindVariable, callback func(reply *sqltypes.Result) error) error
+	// V3 functions.
+	Execute(method string, query string, bindvars map[string]*querypb.BindVariable, rollbackOnError bool, co vtgatepb.CommitOrder) (*sqltypes.Result, error)
+	AutocommitApproval() bool
 
-		// Keyspace ID level functions.
-		ExecuteKeyspaceID(keyspace string, ksid []byte, query string, bindVars map[string]*querypb.BindVariable, rollbackOnError, autocommit bool) (*sqltypes.Result, error)
+	// Shard-level functions.
+	ExecuteMultiShard(rss []*srvtopo.ResolvedShard, queries []*querypb.BoundQuery, rollbackOnError, canAutocommit bool) (*sqltypes.Result, []error)
+	ExecuteStandalone(query string, bindvars map[string]*querypb.BindVariable, rs *srvtopo.ResolvedShard) (*sqltypes.Result, error)
+	StreamExecuteMulti(query string, rss []*srvtopo.ResolvedShard, bindVars []map[string]*querypb.BindVariable, callback func(reply *sqltypes.Result) error) error
 
-		// 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)
+	// Keyspace ID level functions.
+	ExecuteKeyspaceID(keyspace string, ksid []byte, query string, bindVars map[string]*querypb.BindVariable, rollbackOnError, autocommit bool) (*sqltypes.Result, error)
 
-		ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error
+	// 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)
 
-		Session() SessionActions
-	}
-
-	//SessionActions gives primitives ability to interact with the session state
-	SessionActions interface {
-		// RecordWarning stores the given warning in the current session
-		RecordWarning(warning *querypb.QueryWarning)
-
-		SetTarget(target string) error
-		SetUDV(key string, value interface{}) error
-	}
-
-	// Plan represents the execution strategy for a given query.
-	// For now it's a simple wrapper around the real instructions.
-	// An instruction (aka Primitive) is typically a tree where
-	// each node does its part by combining the results of the
-	// sub-nodes.
-	Plan struct {
-		Type                   sqlparser.StatementType // The type of query we have
-		Original               string                  // Original is the original query.
-		Instructions           Primitive               // Instructions contains the instructions needed to fulfil the query.
-		sqlparser.BindVarNeeds                         // Stores BindVars needed to be provided as part of expression rewriting
-
-		mu           sync.Mutex    // Mutex to protect the fields below
-		ExecCount    uint64        // Count of times this plan was executed
-		ExecTime     time.Duration // Total execution time
-		ShardQueries uint64        // Total number of shard queries
-		Rows         uint64        // Total number of rows
-		Errors       uint64        // Total number of errors
-	}
-
-	// Match is used to check if a Primitive matches
-	Match func(node Primitive) bool
-
-	// Primitive is the building block of the engine execution plan. They form a tree structure, where the leaves typically
-	// issue queries to one or more vttablet.
-	// During execution, the Primitive's pass Result objects up the tree structure, until reaching the root,
-	// and its result is passed to the client.
-	Primitive interface {
-		RouteType() string
-		GetKeyspaceName() string
-		GetTableName() string
-		Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error)
-		StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error
-		GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error)
-		NeedsTransaction() bool
-
-		// The inputs to this Primitive
-		Inputs() []Primitive
-
-		// description is the description, sans the inputs, of this Primitive.
-		// to get the plan description with all children, use PrimitiveToPlanDescription()
-		description() PrimitiveDescription
-	}
-
-	// noInputs default implementation for Primitives that are leaves
-	noInputs struct{}
+	SetTarget(target string) error
+	SetUDV(key string, value interface{}) error
 
-	// noTxNeeded is a default implementation for Primitives that don't need transaction handling
-	noTxNeeded struct{}
+	ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error
+}
 
-	// txNeeded is a default implementation for Primitives that need transaction handling
-	txNeeded struct{}
-)
+// Plan represents the execution strategy for a given query.
+// For now it's a simple wrapper around the real instructions.
+// An instruction (aka Primitive) is typically a tree where
+// each node does its part by combining the results of the
+// sub-nodes.
+type Plan struct {
+	Type                   sqlparser.StatementType // The type of query we have
+	Original               string                  // Original is the original query.
+	Instructions           Primitive               // Instructions contains the instructions needed to fulfil the query.
+	sqlparser.BindVarNeeds                         // Stores BindVars needed to be provided as part of expression rewriting
+
+	mu           sync.Mutex    // Mutex to protect the fields below
+	ExecCount    uint64        // Count of times this plan was executed
+	ExecTime     time.Duration // Total execution time
+	ShardQueries uint64        // Total number of shard queries
+	Rows         uint64        // Total number of rows
+	Errors       uint64        // Total number of errors
+}
 
 // AddStats updates the plan execution statistics
 func (p *Plan) AddStats(execCount uint64, execTime time.Duration, shardQueries, rows, errors uint64) {
@@ -161,6 +121,9 @@ func (p *Plan) Stats() (execCount uint64, execTime time.Duration, shardQueries,
 	return
 }
 
+// Match is used to check if a Primitive matches
+type Match func(node Primitive) bool
+
 // Find will return the first Primitive that matches the evaluate function. If no match is found, nil will be returned
 func Find(isMatch Match, start Primitive) Primitive {
 	if isMatch(start) {
@@ -217,15 +180,42 @@ func (p *Plan) MarshalJSON() ([]byte, error) {
 	return json.Marshal(marshalPlan)
 }
 
+// Primitive is the building block of the engine execution plan. They form a tree structure, where the leaves typically
+// issue queries to one or more vttablet.
+// During execution, the Primitive's pass Result objects up the tree structure, until reaching the root,
+// and its result is passed to the client.
+type Primitive interface {
+	RouteType() string
+	GetKeyspaceName() string
+	GetTableName() string
+	Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantfields bool) (*sqltypes.Result, error)
+	StreamExecute(vcursor VCursor, bindVars map[string]*querypb.BindVariable, wantields bool, callback func(*sqltypes.Result) error) error
+	GetFields(vcursor VCursor, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error)
+	NeedsTransaction() bool
+
+	// The inputs to this Primitive
+	Inputs() []Primitive
+
+	// description is the description, sans the inputs, of this Primitive.
+	// to get the plan description with all children, use PrimitiveToPlanDescription()
+	description() PrimitiveDescription
+}
+
+type noInputs struct{}
+
 // Inputs implements no inputs
 func (noInputs) Inputs() []Primitive {
 	return nil
 }
 
+type noTxNeeded struct{}
+
 func (noTxNeeded) NeedsTransaction() bool {
 	return false
 }
 
+type txNeeded struct{}
+
 func (txNeeded) NeedsTransaction() bool {
 	return true
 }
diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go
index 6846ac5498..9fff1e5afe 100644
--- a/go/vt/vtgate/engine/route.go
+++ b/go/vt/vtgate/engine/route.go
@@ -254,7 +254,7 @@ func (route *Route) execute(vcursor VCursor, bindVars map[string]*querypb.BindVa
 			for _, err := range errs {
 				if err != nil {
 					serr := mysql.NewSQLErrorFromError(err).(*mysql.SQLError)
-					vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: uint32(serr.Num), Message: err.Error()})
+					vcursor.RecordWarning(&querypb.QueryWarning{Code: uint32(serr.Num), Message: err.Error()})
 				}
 			}
 			// fall through
diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go
index 42d5a173f2..f4e2b02560 100644
--- a/go/vt/vtgate/engine/set.go
+++ b/go/vt/vtgate/engine/set.go
@@ -17,17 +17,8 @@ limitations under the License.
 package engine
 
 import (
-	"encoding/json"
-	"fmt"
-
-	"vitess.io/vitess/go/mysql"
-
 	"vitess.io/vitess/go/sqltypes"
-	"vitess.io/vitess/go/vt/key"
 	querypb "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"
 )
 
 type (
@@ -49,20 +40,6 @@ type (
 		Name      string
 		PlanValue sqltypes.PlanValue
 	}
-
-	// SysVarIgnore implements the SetOp interface to ignore the settings.
-	SysVarIgnore struct {
-		Name string
-		Expr string
-	}
-
-	// SysVarCheckAndIgnore implements the SetOp interface to check underlying setting and ignore if same.
-	SysVarCheckAndIgnore struct {
-		Name              string
-		Keyspace          *vindexes.Keyspace
-		TargetDestination key.Destination
-		CheckSysVarQuery  string
-	}
 )
 
 var _ Primitive = (*Set)(nil)
@@ -116,18 +93,6 @@ func (s *Set) description() PrimitiveDescription {
 
 var _ SetOp = (*UserDefinedVariable)(nil)
 
-//MarshalJSON provides the type to SetOp for plan json
-func (u *UserDefinedVariable) MarshalJSON() ([]byte, error) {
-	return json.Marshal(struct {
-		Type string
-		UserDefinedVariable
-	}{
-		Type:                "UserDefinedVariable",
-		UserDefinedVariable: *u,
-	})
-
-}
-
 //VariableName implements the SetOp interface method.
 func (u *UserDefinedVariable) VariableName() string {
 	return u.Name
@@ -139,69 +104,5 @@ func (u *UserDefinedVariable) Execute(vcursor VCursor, bindVars map[string]*quer
 	if err != nil {
 		return err
 	}
-	return vcursor.Session().SetUDV(u.Name, value)
-}
-
-var _ SetOp = (*SysVarIgnore)(nil)
-
-//MarshalJSON provides the type to SetOp for plan json
-func (svi *SysVarIgnore) MarshalJSON() ([]byte, error) {
-	return json.Marshal(struct {
-		Type string
-		SysVarIgnore
-	}{
-		Type:         "SysVarIgnore",
-		SysVarIgnore: *svi,
-	})
-
-}
-
-//VariableName implements the SetOp interface method.
-func (svi *SysVarIgnore) VariableName() string {
-	return svi.Name
-}
-
-//Execute implements the SetOp interface method.
-func (svi *SysVarIgnore) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable) error {
-	vcursor.Session().RecordWarning(&querypb.QueryWarning{Code: mysql.ERNotSupportedYet, Message: fmt.Sprintf("Ignored inapplicable SET %v = %v", svi.Name, svi.Expr)})
-	return nil
-}
-
-var _ SetOp = (*SysVarCheckAndIgnore)(nil)
-
-//MarshalJSON provides the type to SetOp for plan json
-func (svci *SysVarCheckAndIgnore) MarshalJSON() ([]byte, error) {
-	return json.Marshal(struct {
-		Type string
-		SysVarCheckAndIgnore
-	}{
-		Type:                 "SysVarCheckAndIgnore",
-		SysVarCheckAndIgnore: *svci,
-	})
-
-}
-
-//VariableName implements the SetOp interface method
-func (svci *SysVarCheckAndIgnore) VariableName() string {
-	return svci.Name
-}
-
-//Execute implements the SetOp interface method
-func (svci *SysVarCheckAndIgnore) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariable) error {
-	rss, _, err := vcursor.ResolveDestinations(svci.Keyspace.Name, nil, []key.Destination{svci.TargetDestination})
-	if err != nil {
-		return vterrors.Wrap(err, "SysVarCheckAndIgnore")
-	}
-
-	if len(rss) != 1 {
-		return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Unexpected error, DestinationKeyspaceID mapping to multiple shards: %v", svci.TargetDestination)
-	}
-	result, err := execShard(vcursor, svci.CheckSysVarQuery, bindVars, rss[0], false /* rollbackOnError */, false /* canAutocommit */)
-	if err != nil {
-		return err
-	}
-	if result.RowsAffected != 1 {
-		return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "Modification not allowed using set construct for: %s", svci.Name)
-	}
-	return nil
+	return vcursor.SetUDV(u.Name, value)
 }
diff --git a/go/vt/vtgate/engine/update_target.go b/go/vt/vtgate/engine/update_target.go
index f637d4166e..f297c43e94 100644
--- a/go/vt/vtgate/engine/update_target.go
+++ b/go/vt/vtgate/engine/update_target.go
@@ -60,7 +60,7 @@ func (updTarget UpdateTarget) GetTableName() string {
 
 // Execute implements the Primitive interface
 func (updTarget UpdateTarget) Execute(vcursor VCursor, bindVars map[string]*query.BindVariable, wantfields bool) (*sqltypes.Result, error) {
-	err := vcursor.Session().SetTarget(updTarget.Target)
+	err := vcursor.SetTarget(updTarget.Target)
 	if err != nil {
 		return nil, err
 	}
diff --git a/go/vt/vtgate/planbuilder/set.go b/go/vt/vtgate/planbuilder/set.go
index 9330e9dd46..4722a405fc 100644
--- a/go/vt/vtgate/planbuilder/set.go
+++ b/go/vt/vtgate/planbuilder/set.go
@@ -17,89 +17,29 @@ limitations under the License.
 package planbuilder
 
 import (
-	"fmt"
-
-	"vitess.io/vitess/go/vt/key"
-	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"
 )
 
-var sysVarPlanningFunc = map[string]func(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error){}
-
-func init() {
-	sysVarPlanningFunc["default_storage_engine"] = buildSetOpIgnore
-	sysVarPlanningFunc["sql_mode"] = buildSetOpCheckAndIgnore
-}
-
 func buildSetPlan(sql string, stmt *sqlparser.Set, vschema ContextVSchema) (engine.Primitive, error) {
 	var setOps []engine.SetOp
-	var setOp engine.SetOp
-	var err error
-
-	if stmt.Scope == sqlparser.GlobalStr {
-		return nil, vterrors.New(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported in set: global")
-	}
 
 	for _, expr := range stmt.Exprs {
 		switch expr.Name.AtCount() {
 		case sqlparser.SingleAt:
-			pv, err := sqlparser.NewPlanValue(expr.Expr)
-			if err != nil {
-				return nil, err
-			}
-			setOp = &engine.UserDefinedVariable{
-				Name:      expr.Name.Lowered(),
-				PlanValue: pv,
-			}
-		case sqlparser.DoubleAt:
-			planFunc, ok := sysVarPlanningFunc[expr.Name.Lowered()]
-			if !ok {
-				return nil, ErrPlanNotSupported
-			}
-			setOp, err = planFunc(expr, vschema)
-			if err != nil {
-				return nil, err
-			}
 		default:
 			return nil, ErrPlanNotSupported
 		}
-		setOps = append(setOps, setOp)
+		pv, err := sqlparser.NewPlanValue(expr.Expr)
+		if err != nil {
+			return nil, err
+		}
+		setOps = append(setOps, &engine.UserDefinedVariable{
+			Name:      expr.Name.Lowered(),
+			PlanValue: pv,
+		})
 	}
 	return &engine.Set{
 		Ops: setOps,
 	}, nil
 }
-
-func buildSetOpIgnore(expr *sqlparser.SetExpr, _ ContextVSchema) (engine.SetOp, error) {
-	buf := sqlparser.NewTrackedBuffer(nil)
-	buf.Myprintf("%v", expr.Expr)
-
-	return &engine.SysVarIgnore{
-		Name: expr.Name.Lowered(),
-		Expr: buf.String(),
-	}, nil
-}
-
-func buildSetOpCheckAndIgnore(expr *sqlparser.SetExpr, vschema ContextVSchema) (engine.SetOp, error) {
-	keyspace, err := vschema.DefaultKeyspace()
-	if err != nil {
-		return nil, err
-	}
-
-	dest := vschema.Destination()
-	if dest == nil {
-		dest = key.DestinationAnyShard{}
-	}
-
-	buf := sqlparser.NewTrackedBuffer(nil)
-	buf.Myprintf("%v", expr.Expr)
-
-	return &engine.SysVarCheckAndIgnore{
-		Name:              expr.Name.Lowered(),
-		Keyspace:          keyspace,
-		TargetDestination: dest,
-		CheckSysVarQuery:  fmt.Sprintf("select 1 from dual where @@%s = %s", expr.Name.Lowered(), buf.String()),
-	}, nil
-}
diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go
index 25ec1cb4ce..02a167846e 100644
--- a/go/vt/vtgate/vcursor_impl.go
+++ b/go/vt/vtgate/vcursor_impl.go
@@ -312,10 +312,6 @@ func (vc *vcursorImpl) ResolveDestinations(keyspace string, ids []*querypb.Value
 	return vc.resolver.ResolveDestinations(vc.ctx, keyspace, vc.tabletType, ids, destinations)
 }
 
-func (vc *vcursorImpl) Session() engine.SessionActions {
-	return vc
-}
-
 func (vc *vcursorImpl) SetTarget(target string) error {
 	keyspace, tabletType, _, err := parseDestinationTarget(target, vc.vschema)
 	if err != nil {


Test patch:
diff --git a/go/jsonutil/json_test.go b/go/jsonutil/json_test.go
new file mode 100644
index 0000000000..d40c26f558
--- /dev/null
+++ b/go/jsonutil/json_test.go
@@ -0,0 +1,89 @@
+package jsonutil
+
+import (
+	"testing"
+)
+
+func TestMarshalNoEscape(t *testing.T) {
+	cases := []struct {
+		name     string
+		v        interface{}
+		expected string
+	}{
+		{
+			name: "normal",
+			v: struct {
+				Usr string
+				Pwd string
+			}{
+				Usr: "vitess",
+				Pwd: "vitess",
+			},
+			expected: "{\"Usr\":\"vitess\",\"Pwd\":\"vitess\"}",
+		},
+		{
+			name: "not exported",
+			v: struct {
+				usr string
+				pwd string
+			}{
+				usr: "vitess",
+				pwd: "vitess",
+			},
+			expected: "{}",
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			json, _ := MarshalNoEscape(c.v)
+			sjson := string(json[:len(json)-1])
+			if sjson != c.expected {
+				t.Errorf("expected: %v, got: %v", c.expected, sjson)
+			}
+		})
+	}
+}
+
+func TestMarshalIndentNoEscape(t *testing.T) {
+	cases := []struct {
+		name     string
+		v        interface{}
+		prefix   string
+		ident    string
+		expected string
+	}{
+		{
+			name: "normal",
+			v: struct {
+				Usr string
+				Pwd string
+			}{
+				Usr: "vitess",
+				Pwd: "vitess",
+			},
+			prefix:   "test",
+			ident:    "\t",
+			expected: "{\ntest\t\"Usr\": \"vitess\",\ntest\t\"Pwd\": \"vitess\"\ntest}",
+		},
+		{
+			name: "not exported",
+			v: struct {
+				usr string
+				pwd string
+			}{
+				usr: "vitess",
+				pwd: "vitess",
+			},
+			expected: "{}",
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			json, _ := MarshalIndentNoEscape(c.v, c.prefix, c.ident)
+			sjson := string(json[:len(json)-1])
+			if sjson != c.expected {
+				t.Errorf("expected: %v, got: %v", c.expected, sjson)
+			}
+		})
+	}
+}
diff --git a/go/test/endtoend/vtgate/setstatement/main_test.go b/go/test/endtoend/vtgate/setstatement/main_test.go
deleted file mode 100644
index 1bb654e694..0000000000
--- a/go/test/endtoend/vtgate/setstatement/main_test.go
+++ /dev/null
@@ -1,101 +0,0 @@
-/*
-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 setstatement
-
-import (
-	"flag"
-	"os"
-	"testing"
-
-	"vitess.io/vitess/go/mysql"
-	"vitess.io/vitess/go/sqltypes"
-	"vitess.io/vitess/go/test/endtoend/cluster"
-)
-
-var (
-	clusterInstance *cluster.LocalProcessCluster
-	keyspaceName    = "ks"
-	cell            = "zone1"
-	hostname        = "localhost"
-	sqlSchema       = `
-	create table test(
-		id bigint,
-		val1 varchar(16),
-		val2 int,
-		val3 float,
-		primary key(id)
-	)Engine=InnoDB;`
-
-	vSchema = `
-		{	
-			"sharded":true,
-			"vindexes": {
-				"hash_index": {
-					"type": "hash"
-				}
-			},	
-			"tables": {
-				"test":{
-					"column_vindexes": [
-						{
-							"column": "id",
-							"name": "hash_index"
-						}
-					]
-				}
-			}
-		}
-	`
-)
-
-func TestMain(m *testing.M) {
-	defer cluster.PanicHandler(nil)
-	flag.Parse()
-
-	exitCode := func() int {
-		clusterInstance = cluster.NewCluster(cell, hostname)
-		defer clusterInstance.Teardown()
-
-		// Start topo server
-		if err := clusterInstance.StartTopo(); err != nil {
-			return 1
-		}
-
-		// Start keyspace
-		keyspace := &cluster.Keyspace{
-			Name:      keyspaceName,
-			SchemaSQL: sqlSchema,
-			VSchema:   vSchema,
-		}
-		if err := clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 1, false); err != nil {
-			return 1
-		}
-
-		// Start vtgate
-		if err := clusterInstance.StartVtgate(); err != nil {
-			return 1
-		}
-
-		return m.Run()
-	}()
-	os.Exit(exitCode)
-}
-
-func exec(t *testing.T, conn *mysql.Conn, query string) (*sqltypes.Result, error) {
-	t.Helper()
-	return conn.ExecuteFetch(query, 1000, true)
-}
diff --git a/go/test/endtoend/vtgate/setstatement/sysvar_test.go b/go/test/endtoend/vtgate/setstatement/sysvar_test.go
deleted file mode 100644
index 3db7aaadb8..0000000000
--- a/go/test/endtoend/vtgate/setstatement/sysvar_test.go
+++ /dev/null
@@ -1,89 +0,0 @@
-/*
-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 setstatement
-
-import (
-	"context"
-	"fmt"
-	"testing"
-
-	"github.com/google/go-cmp/cmp"
-	"github.com/stretchr/testify/require"
-
-	"vitess.io/vitess/go/mysql"
-	"vitess.io/vitess/go/test/endtoend/cluster"
-)
-
-func TestSetSysVar(t *testing.T) {
-	defer cluster.PanicHandler(t)
-	ctx := context.Background()
-	vtParams := mysql.ConnParams{
-		Host: "localhost",
-		Port: clusterInstance.VtgateMySQLPort,
-	}
-	type queriesWithExpectations struct {
-		query           string
-		expectedRows    string
-		rowsAffected    int
-		errMsg          string
-		expectedWarning string
-	}
-
-	queries := []queriesWithExpectations{{
-		query:        `set @@default_storage_engine = INNODB`,
-		expectedRows: ``, rowsAffected: 0,
-		expectedWarning: "[[VARCHAR(\"Warning\") UINT16(1235) VARCHAR(\"Ignored inapplicable SET default_storage_engine = INNODB\")]]",
-	}, {
-		query:        `set @@sql_mode = @@sql_mode`,
-		expectedRows: ``, rowsAffected: 0,
-	}, {
-		query:        `set @@sql_mode = concat(@@sql_mode,"")`,
-		expectedRows: ``, rowsAffected: 0,
-	}, {
-		query:  `set @@sql_mode = concat(@@sql_mode,"ALLOW_INVALID_DATES")`,
-		errMsg: "Modification not allowed using set construct for: sql_mode",
-	}}
-
-	conn, err := mysql.Connect(ctx, &vtParams)
-	require.NoError(t, err)
-	defer conn.Close()
-
-	for i, q := range queries {
-		t.Run(fmt.Sprintf("%d-%s", i, q.query), func(t *testing.T) {
-			qr, err := exec(t, conn, q.query)
-			if q.errMsg != "" {
-				require.Contains(t, err.Error(), q.errMsg)
-			} else {
-				require.NoError(t, err)
-				require.Equal(t, uint64(q.rowsAffected), qr.RowsAffected, "rows affected wrong for query: %s", q.query)
-				if q.expectedRows != "" {
-					result := fmt.Sprintf("%v", qr.Rows)
-					if diff := cmp.Diff(q.expectedRows, result); diff != "" {
-						t.Errorf("%s\nfor query: %s", diff, q.query)
-					}
-				}
-				if q.expectedWarning != "" {
-					qr, err := exec(t, conn, "show warnings")
-					require.NoError(t, err)
-					if got, want := fmt.Sprintf("%v", qr.Rows), q.expectedWarning; got != want {
-						t.Errorf("select:\n%v want\n%v", got, want)
-					}
-				}
-			}
-		})
-	}
-}
diff --git a/go/test/endtoend/vtgate/setstatement/udv_test.go b/go/test/endtoend/vtgate/setstatement/udv_test.go
index 8e2cff6e72..86baee4adc 100644
--- a/go/test/endtoend/vtgate/setstatement/udv_test.go
+++ b/go/test/endtoend/vtgate/setstatement/udv_test.go
@@ -1,34 +1,99 @@
-/*
-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 setstatement
 
 import (
 	"context"
+	"flag"
 	"fmt"
+	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+
 	"github.com/stretchr/testify/require"
 
 	"vitess.io/vitess/go/mysql"
+	"vitess.io/vitess/go/sqltypes"
 	"vitess.io/vitess/go/test/endtoend/cluster"
 )
 
-func TestSetUDV(t *testing.T) {
+var (
+	clusterInstance *cluster.LocalProcessCluster
+	keyspaceName    = "ks"
+	cell            = "zone1"
+	hostname        = "localhost"
+	sqlSchema       = `
+	create table test(
+		id bigint,
+		val1 varchar(16),
+		val2 int,
+		val3 float,
+		primary key(id)
+	)Engine=InnoDB;`
+
+	vSchema = `
+		{	
+			"sharded":true,
+			"vindexes": {
+				"hash_index": {
+					"type": "hash"
+				}
+			},	
+			"tables": {
+				"test":{
+					"column_vindexes": [
+						{
+							"column": "id",
+							"name": "hash_index"
+						}
+					]
+				}
+			}
+		}
+	`
+)
+
+func TestMain(m *testing.M) {
+	defer cluster.PanicHandler(nil)
+	flag.Parse()
+
+	exitCode := func() int {
+		clusterInstance = cluster.NewCluster(cell, hostname)
+		defer clusterInstance.Teardown()
+
+		// Start topo server
+		if err := clusterInstance.StartTopo(); err != nil {
+			return 1
+		}
+
+		// Start keyspace
+		keyspace := &cluster.Keyspace{
+			Name:      keyspaceName,
+			SchemaSQL: sqlSchema,
+			VSchema:   vSchema,
+		}
+		if err := clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 1, false); err != nil {
+			return 1
+		}
+
+		// Start vtgate
+		if err := clusterInstance.StartVtgate(); err != nil {
+			return 1
+		}
+
+		return m.Run()
+	}()
+	os.Exit(exitCode)
+}
+
+func exec(t *testing.T, conn *mysql.Conn, query string) *sqltypes.Result {
+	t.Helper()
+	qr, err := conn.ExecuteFetch(query, 1000, true)
+	require.Nil(t, err)
+	return qr
+}
+
+func TestSet(t *testing.T) {
 	defer cluster.PanicHandler(t)
 	ctx := context.Background()
 	vtParams := mysql.ConnParams{
@@ -85,9 +150,8 @@ func TestSetUDV(t *testing.T) {
 
 	for i, q := range queries {
 		t.Run(fmt.Sprintf("%d-%s", i, q.query), func(t *testing.T) {
-			qr, err := exec(t, conn, q.query)
-			require.NoError(t, err)
-			require.Equal(t, uint64(q.rowsAffected), qr.RowsAffected, "rows affected wrong for query: %s", q.query)
+			qr := exec(t, conn, q.query)
+			assert.Equal(t, uint64(q.rowsAffected), qr.RowsAffected, "rows affected wrong for query: %s", q.query)
 			if q.expectedRows != "" {
 				result := fmt.Sprintf("%v", qr.Rows)
 				if diff := cmp.Diff(q.expectedRows, result); diff != "" {
diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go
index e9837e9f50..2b7acc6d80 100644
--- a/go/vt/vtgate/engine/fake_vcursor_test.go
+++ b/go/vt/vtgate/engine/fake_vcursor_test.go
@@ -54,9 +54,7 @@ func (t noopVCursor) SetUDV(key string, value interface{}) error {
 func (t noopVCursor) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL) error {
 	panic("implement me")
 }
-func (t noopVCursor) Session() SessionActions {
-	return t
-}
+
 func (t noopVCursor) SetTarget(target string) error {
 	panic("implement me")
 }
@@ -138,10 +136,6 @@ func (f *loggingVCursor) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.D
 	panic("implement me")
 }
 
-func (f *loggingVCursor) Session() SessionActions {
-	return f
-}
-
 func (f *loggingVCursor) SetTarget(target string) error {
 	f.log = append(f.log, fmt.Sprintf("Target set to %s", target))
 	return nil
diff --git a/go/vt/vtgate/engine/send_test.go b/go/vt/vtgate/engine/send_test.go
index 4604ce0e48..5c8bf8efa0 100644
--- a/go/vt/vtgate/engine/send_test.go
+++ b/go/vt/vtgate/engine/send_test.go
@@ -1,19 +1,3 @@
-/*
-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/engine/set_test.go b/go/vt/vtgate/engine/set_test.go
deleted file mode 100644
index a243445b4c..0000000000
--- a/go/vt/vtgate/engine/set_test.go
+++ /dev/null
@@ -1,195 +0,0 @@
-/*
-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 (
-	"testing"
-
-	"vitess.io/vitess/go/sqltypes"
-	"vitess.io/vitess/go/vt/key"
-	"vitess.io/vitess/go/vt/vtgate/vindexes"
-
-	"github.com/stretchr/testify/require"
-
-	querypb "vitess.io/vitess/go/vt/proto/query"
-)
-
-func TestSetTable(t *testing.T) {
-	type testCase struct {
-		testName         string
-		setOps           []SetOp
-		qr               []*sqltypes.Result
-		expectedQueryLog []string
-		expectedWarning  []*querypb.QueryWarning
-		expectedError    string
-	}
-
-	tests := []testCase{
-		{
-			testName:         "nil set ops",
-			expectedQueryLog: []string{},
-		},
-		{
-			testName: "udv",
-			setOps: []SetOp{
-				&UserDefinedVariable{
-					Name: "x",
-					PlanValue: sqltypes.PlanValue{
-						Value: sqltypes.NewInt64(42),
-					},
-				},
-			},
-			expectedQueryLog: []string{
-				`UDV set with (x,INT64(42))`,
-			},
-		},
-		{
-			testName: "sysvar ignore",
-			setOps: []SetOp{
-				&SysVarIgnore{
-					Name: "x",
-					Expr: "42",
-				},
-			},
-			expectedWarning: []*querypb.QueryWarning{
-				{Code: 1235, Message: "Ignored inapplicable SET x = 42"},
-			},
-		},
-		{
-			testName: "sysvar check and ignore",
-			setOps: []SetOp{
-				&SysVarCheckAndIgnore{
-					Name: "x",
-					Keyspace: &vindexes.Keyspace{
-						Name:    "ks",
-						Sharded: true,
-					},
-					TargetDestination: key.DestinationAnyShard{},
-					CheckSysVarQuery:  "dummy_query",
-				},
-			},
-			qr: []*sqltypes.Result{sqltypes.MakeTestResult(
-				sqltypes.MakeTestFields(
-					"id",
-					"int64",
-				),
-				"1",
-			)},
-			expectedQueryLog: []string{
-				`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
-				`ExecuteMultiShard ks.-20: dummy_query {} false false`,
-			},
-		},
-		{
-			testName: "sysvar check and error",
-			setOps: []SetOp{
-				&SysVarCheckAndIgnore{
-					Name: "x",
-					Keyspace: &vindexes.Keyspace{
-						Name:    "ks",
-						Sharded: true,
-					},
-					TargetDestination: key.DestinationAnyShard{},
-					CheckSysVarQuery:  "dummy_query",
-				},
-			},
-			expectedQueryLog: []string{
-				`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
-				`ExecuteMultiShard ks.-20: dummy_query {} false false`,
-			},
-			expectedError: "Modification not allowed using set construct for: x",
-		},
-		{
-			testName: "sysvar checkAndIgnore multi destination error",
-			setOps: []SetOp{
-				&SysVarCheckAndIgnore{
-					Name: "x",
-					Keyspace: &vindexes.Keyspace{
-						Name:    "ks",
-						Sharded: true,
-					},
-					TargetDestination: key.DestinationAllShards{},
-					CheckSysVarQuery:  "dummy_query",
-				},
-			},
-			expectedQueryLog: []string{
-				`ResolveDestinations ks [] Destinations:DestinationAllShards()`,
-			},
-			expectedError: "Unexpected error, DestinationKeyspaceID mapping to multiple shards: DestinationAllShards()",
-		},
-		{
-			testName: "udv_ignr_chignr",
-			setOps: []SetOp{
-				&UserDefinedVariable{
-					Name: "x",
-					PlanValue: sqltypes.PlanValue{
-						Value: sqltypes.NewInt64(1),
-					},
-				},
-				&SysVarIgnore{
-					Name: "y",
-					Expr: "2",
-				},
-				&SysVarCheckAndIgnore{
-					Name: "z",
-					Keyspace: &vindexes.Keyspace{
-						Name:    "ks",
-						Sharded: true,
-					},
-					TargetDestination: key.DestinationAnyShard{},
-					CheckSysVarQuery:  "dummy_query",
-				},
-			},
-			expectedQueryLog: []string{
-				`UDV set with (x,INT64(1))`,
-				`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
-				`ExecuteMultiShard ks.-20: dummy_query {} false false`,
-			},
-			expectedWarning: []*querypb.QueryWarning{
-				{Code: 1235, Message: "Ignored inapplicable SET y = 2"},
-			},
-			qr: []*sqltypes.Result{sqltypes.MakeTestResult(
-				sqltypes.MakeTestFields(
-					"id",
-					"int64",
-				),
-				"1",
-			)},
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.testName, func(t *testing.T) {
-			set := &Set{
-				Ops: tc.setOps,
-			}
-			vc := &loggingVCursor{
-				shards:  []string{"-20", "20-"},
-				results: tc.qr,
-			}
-			_, err := set.Execute(vc, map[string]*querypb.BindVariable{}, false)
-			if tc.expectedError == "" {
-				require.NoError(t, err)
-			} else {
-				require.EqualError(t, err, tc.expectedError)
-			}
-
-			vc.ExpectLog(t, tc.expectedQueryLog)
-			vc.ExpectWarnings(t, tc.expectedWarning)
-		})
-	}
-}
diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go
index 6cf2316896..5dc1423de3 100644
--- a/go/vt/vtgate/executor_set_test.go
+++ b/go/vt/vtgate/executor_set_test.go
@@ -117,7 +117,7 @@ func TestExecutorSet(t *testing.T) {
 		err: "unsupported in set: global",
 	}, {
 		in:  "set global @@session.client_found_rows = 1",
-		err: "unsupported in set: global",
+		err: "unsupported in set: mixed using of variable scope",
 	}, {
 		in:  "set client_found_rows = 'aa'",
 		err: "unexpected value type for client_found_rows: string",
diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go
index 27f7105409..509e14b37f 100644
--- a/go/vt/vtgate/planbuilder/plan_test.go
+++ b/go/vt/vtgate/planbuilder/plan_test.go
@@ -170,7 +170,6 @@ func TestPlan(t *testing.T) {
 	testFile(t, "memory_sort_cases.txt", testOutputTempDir, vschemaWrapper)
 	testFile(t, "use_cases.txt", testOutputTempDir, vschemaWrapper)
 	testFile(t, "set_cases.txt", testOutputTempDir, vschemaWrapper)
-	testFile(t, "set_sysvar_cases.txt", testOutputTempDir, vschemaWrapper)
 }
 
 func TestOne(t *testing.T) {
diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_cases.txt
index b790194c7c..99d4f72e92 100644
--- a/go/vt/vtgate/planbuilder/testdata/set_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/set_cases.txt
@@ -8,7 +8,6 @@
     "Variant": "",
     "Ops": [
       {
-        "Type": "UserDefinedVariable",
         "Name": "foo",
         "PlanValue": 42
       }
@@ -26,12 +25,10 @@
     "Variant": "",
     "Ops": [
       {
-        "Type": "UserDefinedVariable",
         "Name": "foo",
         "PlanValue": 42
       },
       {
-        "Type": "UserDefinedVariable",
         "Name": "bar",
         "PlanValue": ":__vtudvfoo"
       }
diff --git a/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt b/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt
deleted file mode 100644
index 5d554ab27a..0000000000
--- a/go/vt/vtgate/planbuilder/testdata/set_sysvar_cases.txt
+++ /dev/null
@@ -1,44 +0,0 @@
-# set ignore plan
-"set @@default_storage_engine = 'DONOTCHANGEME'"
-{
-  "QueryType": "SET",
-  "Original": "set @@default_storage_engine = 'DONOTCHANGEME'",
-  "Instructions": {
-    "OperatorType": "Set",
-    "Variant": "",
-    "Ops": [
-      {
-        "Type": "SysVarIgnore",
-        "Name": "default_storage_engine",
-        "Expr": "'DONOTCHANGEME'"
-      }
-    ]
-  }
-}
-
-# set check and ignore plan
-"set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')"
-{
-  "QueryType": "SET",
-  "Original": "set @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')",
-  "Instructions": {
-    "OperatorType": "Set",
-    "Variant": "",
-    "Ops": [
-      {
-        "Type": "SysVarCheckAndIgnore",
-        "Name": "sql_mode",
-        "Keyspace": {
-          "Name": "main",
-          "Sharded": false
-        },
-        "TargetDestination": {},
-        "CheckSysVarQuery": "select 1 from dual where @@sql_mode = concat(@@sql_mode, ',NO_AUTO_CREATE_USER')"
-      }
-    ]
-  }
-}
-
-# set plan building not supported
-"set @@innodb_strict_mode = OFF"
-"plan building not supported"
diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
index d2bc6dac7d..b9173fb409 100644
--- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.txt
@@ -11,7 +11,7 @@
 "expression is too complex ':__vtudvfoo + 1'"
 
 # set user defined and system variable
-"set @foo = 42, @bar = @foo, @@wait_timeout = 28800"
+"set @foo = 42, @bar = @foo, @@sql_mode = @@sql_mode"
 "plan building not supported"
 
 # SHOW